Editor: Layer renaming #11477
Merged
Editor: Layer renaming #11477
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mariana-k
added
Type: Enhancement
mariana-k
added
Group: Canvas
Group: Workspace
barklund
previously approved these changes
May 13, 2022
There was a problem hiding this comment.
This is really good, but just has a few issues.
- The current layer name should be extracted from the element type, but this work is already done in Editor: Generalized layer name #11485, so feel free to merge that in once merged to main (@merapi will probably do that first thing Friday)
- The state of whether a layer is renamable should not exist in the story provider, but in some other location. Also, only one layer should ever be renamable.
- If a layer is renamed, the input should be focused (and the layer dialog should be open to do that). It should never be possible to have the rename input unfocused at all.
I have suggested fixes for all these inline below. I will tentatively approve pending the implementation of the above changes.
packages/story-editor/src/components/panels/design/layer/layer.js
Outdated
Show resolved
Hide resolved
packages/story-editor/src/components/panels/design/layer/layer.js
Outdated
Show resolved
Hide resolved
packages/story-editor/src/components/panels/design/layer/useLayerSelection.js
Outdated
Show resolved
Hide resolved
packages/story-editor/src/components/panels/design/layer/useLayerSelection.js
Outdated
Show resolved
Hide resolved
packages/story-editor/src/app/rightClickMenu/items/layerName.js
Outdated
Show resolved
Hide resolved
|
Size Change: +621 B (0%) Total Size: 2.63 MB
ℹ️ View Unchanged
|
|
Plugin builds for 66c66fa are ready 🛎️!
|
|
This pull request introduces 1 alert when merging bdc4fc0 into 1a1ae11 - view on LGTM.com new alerts:
|
merapi
suggested changes
May 19, 2022
There was a problem hiding this comment.
I'm reporting what I see in the current version:
-
After entering the edit mode (via double click) you cannot select a part of the text, instead it cancels the edit mode.
-
Go into edit mode, type something and cancel the edit mode (press Escape). The layer name is reverted to the old name - good, but when you enter the edit mode again you see the old (canceled) value.
-
This^ should look like it looked before (and in Figma). Those extra icons are unnecessary (I deleted my layer by accident when selecting text). -
If the Background layer name cannot be changed then you shouldn't be able to enter the edit mode on that layer.
merapi
previously approved these changes
May 26, 2022
There was a problem hiding this comment.
This looks good now 👍🏻
Nit: handling empty layer name.
Right now if you add layer media/unsplash:gl8BgGhvVdg, rename it to layer1, then rename it to empty string, it will become media/unsplash:gl8BgGhvVdg, but it should probably become layer1? Then if you enter the edit mode again - it will show an empty input (should always be the layer name visible before entering the edit mode?).
And a thing to consider: if we remove the margin-left: 2px, then the layer name will not shift when you enter the edit mode - it looks better IMO (even if it's 2px more over the icon).
merapi
approved these changes
Jun 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
Editing layers names
Summary
Editing of the layers names can be enabled by:
A name is saved by clicking Enter, clicking outside of the layer button (Blur).
The editing is canceled without saving a new name buy clicking Escape.
Relevant Technical Choices
layerName property was added to the element object.
To-do
User-facing changes
Testing Instructions
This PR can be tested by following these steps:
Renaming successful scenario:
Renaming canceled scenario:
5. Repeat 1 - 4, but click Escape button instead of Enter -> the layer name should not be updated.
Note: when an input field is visible, layer action icons should be hidden.
Checklist
Type: XYZlabel to the PRFixes #9248