Disable image "Become background" on standard cover pages (BL-16130)#7824
Disable image "Become background" on standard cover pages (BL-16130)#7824StephenMcConnel wants to merge 1 commit intomasterfrom
Conversation
|
Let's hold off on this for a bit until we see if we can merge in the canvas refactor, which already has this change but in a different place. |
JohnThomson
left a comment
There was a problem hiding this comment.
@JohnThomson made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on StephenMcConnel).
src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx line 1710 at r1 (raw file):
!customLayoutablePage.classList.contains("bloom-customLayout")) ?? false;
We should think carefully about exactly what circumstances make this command inappropriate. In practice it might not make much difference, but we should try to make the code reflect our thinking. Currently, we have [data-custom-layout-id] only on two page types, and only one of them, the front cover, normally has an image that is a canvas element. In standard mode, that image shouldn't have this command (or it should be disabled). But does it make more sense to say we can't do this
- because we're on a standard cover? But it would be OK to do it if a standard credits page had an image, as long as that page can't be made custom? I would think this should rather be forbidden for any page which doesn't allow overlays, which at the moment is all xmatter pages EXCEPT if they ARE in custom mode. So the first condition should be looking for xmatter pages, not just pages that could be made custom, and isStandardCoverPage would perhaps better be called something like pageAllowsCanvasElements.
- because the image is ALREADY a background image? On this theory it should be disabled/hidden even on content pages if the current element already has the class bloom-backgroundImage. However, this may be undesirable if people become used to using 'become background image' instead of 'expand to fill space', since that command now does both. Question for JohnH?
src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx line 1830 at r1 (raw file):
l10nId: "EditTab.Toolbox.ComicTool.Options.BecomeBackground", english: "Become Background", disabled: isStandardCoverPage,
Do we want to disable it or hide it altogether? The card says it "should not have" the command, which to me suggests hiding it altogether.
JohnThomson
left a comment
There was a problem hiding this comment.
We've now decided to hold off on the canvas refactor, so we should proceed with this.
@JohnThomson made 1 comment.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on StephenMcConnel).
This change is