diff --git a/src/containers/backdrop-library.jsx b/src/containers/backdrop-library.jsx index 5c2a315dcf9df8fdd70a42c266ffa85c4dded334..91b09e2140117bb903c5a30a0707d9a7e69b6edd 100644 --- a/src/containers/backdrop-library.jsx +++ b/src/containers/backdrop-library.jsx @@ -33,7 +33,7 @@ class BackdropLibrary extends React.Component { bitmapResolution: item.info.length > 2 ? item.info[2] : 1, skinId: null }; - this.props.vm.setEditingTarget(this.props.stageID); + // Do not switch to stage, just add the backdrop this.props.vm.addBackdrop(item.md5, vmBackdrop); } render () { @@ -53,17 +53,7 @@ class BackdropLibrary extends React.Component { BackdropLibrary.propTypes = { intl: intlShape.isRequired, onRequestClose: PropTypes.func, - stageID: PropTypes.string.isRequired, vm: PropTypes.instanceOf(VM).isRequired }; -const mapStateToProps = state => ({ - stageID: state.scratchGui.targets.stage.id -}); - -const mapDispatchToProps = () => ({}); - -export default injectIntl(connect( - mapStateToProps, - mapDispatchToProps -)(BackdropLibrary)); +export default injectIntl(BackdropLibrary); diff --git a/src/containers/stage-selector.jsx b/src/containers/stage-selector.jsx index 4babbc014233a8bc817e2095a2fb3cd6f9463697..d091c9587438461d3864333c6b074fc5b9eebf4d 100644 --- a/src/containers/stage-selector.jsx +++ b/src/containers/stage-selector.jsx @@ -50,7 +50,7 @@ class StageSelector extends React.Component { 'setFileInput' ]); } - addBackdropFromLibraryItem (item) { + addBackdropFromLibraryItem (item, shouldActivateTab = true) { const vmBackdrop = { name: item.name, md5: item.md5, @@ -59,25 +59,30 @@ class StageSelector extends React.Component { bitmapResolution: item.info.length > 2 ? item.info[2] : 1, skinId: null }; - this.handleNewBackdrop(vmBackdrop); + this.handleNewBackdrop(vmBackdrop, shouldActivateTab); } handleClick () { this.props.onSelect(this.props.id); } - handleNewBackdrop (backdrops_) { + handleNewBackdrop (backdrops_, shouldActivateTab = true) { const backdrops = Array.isArray(backdrops_) ? backdrops_ : [backdrops_]; return Promise.all(backdrops.map(backdrop => this.props.vm.addBackdrop(backdrop.md5, backdrop) - )).then(() => - this.props.onActivateTab(COSTUMES_TAB_INDEX) - ); + )).then(() => { + if (shouldActivateTab) { + return this.props.onActivateTab(COSTUMES_TAB_INDEX); + } + }); } - handleSurpriseBackdrop () { + handleSurpriseBackdrop (e) { + e.stopPropagation(); // Prevent click from falling through to selecting stage. // @todo should this not add a backdrop you already have? const item = backdropLibraryContent[Math.floor(Math.random() * backdropLibraryContent.length)]; - this.addBackdropFromLibraryItem(item); + this.addBackdropFromLibraryItem(item, false); } - handleEmptyBackdrop () { + handleEmptyBackdrop (e) { + e.stopPropagation(); // Prevent click from falling through to stage selector, select it manually below + this.props.vm.setEditingTarget(this.props.id); this.handleNewBackdrop(emptyCostume(this.props.intl.formatMessage(sharedMessages.backdrop, {index: 1}))); } handleBackdropUpload (e) { @@ -85,6 +90,7 @@ class StageSelector extends React.Component { this.props.onShowImporting(); handleFileUpload(e.target, (buffer, fileType, fileName, fileIndex, fileCount) => { costumeUpload(buffer, fileType, storage, vmCostumes => { + this.props.vm.setEditingTarget(this.props.id); vmCostumes.forEach((costume, i) => { costume.name = `${fileName}${i ? i + 1 : ''}`; }); @@ -96,7 +102,8 @@ class StageSelector extends React.Component { }, this.props.onCloseImporting); }, this.props.onCloseImporting); } - handleFileUploadClick () { + handleFileUploadClick (e) { + e.stopPropagation(); // Prevent click from selecting the stage, that is handled manually in backdrop upload this.fileInput.click(); } handleMouseEnter () { diff --git a/test/integration/backdrops.test.js b/test/integration/backdrops.test.js index 3d2f64afa29c56a4e9830a9a12827fe15c78653f..95a268b76c3ee4016013c110d012f555be62717f 100644 --- a/test/integration/backdrops.test.js +++ b/test/integration/backdrops.test.js @@ -25,7 +25,7 @@ describe('Working with backdrops', () => { await driver.quit(); }); - test('Adding a backdrop from the library', async () => { + test('Adding a backdrop from the library should not switch to stage', async () => { await loadUri(uri); // Start on the sounds tab of sprite1 to test switching behavior @@ -37,12 +37,11 @@ describe('Working with backdrops', () => { await el.sendKeys('blue'); await clickText('Blue Sky'); // Adds the backdrop - // Make sure the stage is selected and the sound tab remains selected. - // This is different from Scratch2 which selected backdrop tab automatically - // See issue #3500 - await clickText('pop', scope.soundsTab); + // Make sure the sprite is still selected, and that the tab has not changed + await clickText('Meow', scope.soundsTab); // Make sure the backdrop was actually added by going to the backdrops tab + await clickXpath('//span[text()="Stage"]'); await clickText('Backdrops'); await clickText('Blue Sky', scope.costumesTab); @@ -50,7 +49,48 @@ describe('Working with backdrops', () => { await expect(logs).toEqual([]); }); - test('Adding multiple backdrops at the same time', async () => { + test('Adding backdrop via paint should switch to stage', async () => { + await loadUri(uri); + + const buttonXpath = '//button[@aria-label="Choose a Backdrop"]'; + const paintXpath = `${buttonXpath}/following-sibling::div//button[@aria-label="Paint"]`; + + const el = await findByXpath(buttonXpath); + await driver.actions().mouseMove(el) + .perform(); + await driver.sleep(500); // Wait for thermometer menu to come up + await clickXpath(paintXpath); + + // Stage should become selected and costume tab activated + await findByText('backdrop2', scope.costumesTab); + + const logs = await getLogs(); + await expect(logs).toEqual([]); + }); + + test('Adding backdrop via surprise should not switch to stage', async () => { + await loadUri(uri); + + // Start on the sounds tab of sprite1 to test switching behavior + await clickText('Sounds'); + + const buttonXpath = '//button[@aria-label="Choose a Backdrop"]'; + const surpriseXpath = `${buttonXpath}/following-sibling::div//button[@aria-label="Surprise"]`; + + const el = await findByXpath(buttonXpath); + await driver.actions().mouseMove(el) + .perform(); + await driver.sleep(500); // Wait for thermometer menu to come up + await clickXpath(surpriseXpath); + + // Make sure the sprite is still selected, and that the tab has not changed + await clickText('Meow', scope.soundsTab); + + const logs = await getLogs(); + await expect(logs).toEqual([]); + }); + + test('Adding multiple backdrops from file should switch to stage', async () => { const files = [ path.resolve(__dirname, '../fixtures/gh-3582-png.png'), path.resolve(__dirname, '../fixtures/100-100.svg') @@ -67,7 +107,7 @@ describe('Working with backdrops', () => { const input = await findByXpath(fileXpath); await input.sendKeys(files.join('\n')); - await clickXpath('//span[text()="Stage"]'); + // Should have been switched to stage/costume tab already await findByText('gh-3582-png', scope.costumesTab); await findByText('100-100', scope.costumesTab);