Remove unused props from default templates and save requests #13221
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
ayushnirwal
changed the title
current and selection from default templates and remove basedOn prop current and selection props from default templates and remove basedOn prop from save requests
ayushnirwal
changed the title
current and selection props from default templates and remove basedOn prop from save requestsbasedOn prop from save requests
|
Size Change: -23.4 kB (-1%) Total Size: 2.71 MB
ℹ️ View Unchanged
|
|
Plugin builds for da96b72 are ready 🛎️!
|
Let's also remove them in
With that in place we should properly refactor Thoughts? |
Is there no use case of saving |
|
That's what I don't know :-) It's been a while since this was added, so I am lacking the context. |
Found one use case here. web-stories-wp/packages/elements/src/utils/duplicateElement.ts Lines 76 to 88 in 80d3192 These lines of code are responsible for calculating the positional offset based on the number of duplicates. I think |
So the more duplicates an element has, the bigger is the offset 🤔 Interesting way to achieve this effect: Screen.Recording.2023-04-27.at.11.36.25.movEspecially since it doesn't really account for the positions of all the existing elements, leading to unexpected offsets like this: Screen.Recording.2023-04-27.at.11.37.38.movI think in the end it comes all back to #1206, where we originally envisioned nice staggering behavior when inserting multiple elements, leading to solutions like this and also #4218 Can we maybe find a simpler staggering behavior for duplicating elements without having to use |
|
@swissspidy Since the pasted element/layer/group becomes selected, offset can be calculated off of that. |
swissspidy
approved these changes
May 9, 2023
swissspidy
added
Group: Canvas
Type: Code Quality
…GoogleForCreators/web-stories-wp into code/12262-remove-some-persistant-props
swissspidy
reviewed
May 10, 2023
swissspidy
reviewed
May 10, 2023
swissspidy
changed the title
basedOn prop from save requests
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
Summary
current,selectionandstoryprops from default template.getStoryPropsToSaveto ignorebasedOnproperty in elements.StorySaveData.basedOnproperty.Relevant Technical Choices
To-do
basedOnfrom the types.getStroryPropsToSaveto not deletebasedOnattribute.User-facing changes
Testing Instructions
This PR can be tested by following these steps:
Reviews
Does this PR have a security-related impact?
Does this PR change what data or activity we track or use?
Does this PR have a legal-related impact?
Checklist
Type: XYZlabel to the PRFixes #12262