-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Experimental widgets screen] Remove "id" attribute of the legacy-widget block #24817
Conversation
Size Change: +39 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally and it works. Also code looks mildly complex if you run into it without this context (especially getWidgetIdForClientId
) but I don't have another way or solution :) 🚢
@draganescu agreed, at the same time it's the same pattern as on the navigation screen - to make this screen truly extendable there needs to be an API to match widget Id with it's corresponding client Id. The good news is that if it ends up being confusing, it should be pretty easy to refactor before the beta, it's a pretty small part of the system. |
Description
As @noisysocks mentioned in his comment:
The culprit is that widgets are saved using data from their attributes. ID is one such attribute, and once the block is duplicated there are two blocks with the same ID. This PR removes the ID attribute entirely, replacing it with pulling the widget ID correlated with the block ID from the data store. It also removes the dependency of the save function on the
number
block attribute. Note that thenumber
attribute could potentially be removed entirely in a subsequent PR.How has this been tested?
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: