Migrate hidden block types (block manager data) to new preferences packages #39132
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
|
Size Change: +3.13 kB (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
talldan
commented
Mar 1, 2022
| ); | ||
| } | ||
|
|
||
| const preferences = getPreferences( state ); |
There was a problem hiding this comment.
I need to do a bit more work on back compat here and make sure getPreferences has any migrated preferences merged into it.
There was a problem hiding this comment.
Would we then deprecate getPreference?
There was a problem hiding this comment.
Once everything is migrated I think it can be deprecated, yep 😄
|
Looking good. I agree with this approach as opposed to the alternative you described. I think that:
Fix the tests and I'll review properly 😀 |
8 tasks
Base automatically changed from
migrate/edit-post-to-use-new-preferences
to
trunk
March 2, 2022 04:39
d8c4105 to
9c3d207
Compare
9c3d207 to
6b2e687
Compare
talldan
commented
Mar 2, 2022
| } ); | ||
| } ); | ||
|
|
||
| describe( '__experimentalUpdateLocalAutosaveInterval', () => { |
There was a problem hiding this comment.
I've added tests for all the other preferences to bring some confidence that the changes to getPreferences and getPreference were correct. Plus they'll help when I come to migrate the other preferences.
Agree with that, I immediately went looking for it. 😄 It does mean a selector that doesn't really do much other than get something from the preferences store, but that's probably ok. This is also going to set the pattern for every other editor that adds the block manager, which is the only other thing that concerns me. |
|
I believe I've addressed all the feedback. Registry selectors are a little tricky to test, so I've mostly tested them within the action tests. I think it's enough coverage |
18 tasks
noisysocks
approved these changes
Mar 3, 2022
| ### getHiddenBlockTypes | ||
|
|
||
| Registry selector that gets the hidden block types from the preferences | ||
| store. |
There was a problem hiding this comment.
I think the descriptions for the selector and two actions are a little too descriptive of the implementation. A plugin author probably doesn't care how it's implemented, just what happens when e.g. hideBlockTypes is dispatched, i.e. the block is hidden from the inserters.
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.
Description
This PR is the next in a series of PRs migrating our editor preferences to use the new preferences package.
It tackles the
hiddenBlockTypesstate in the post editor that's used to drive the block manager, and should enable the block manager to be used within other editors (if we can figure out a nice way to migrate the component).While #39115 handles all the boolean 'feature' preferences that are easy to migrate, the
hiddenBlockTypesstate has its own reducer, so I think it makes sense to have a separate PR to give this more scrutiny.There are probably a few different ways to do this, but I've chosen to keep the code roughly equivalent, basically moving the
hiddenBlockTypereducer logic into the actions.An alternative I considered
An alternative could be to have an individual preference in the preferences store for each block and use a compound key, something like:
This would simplify the actions, but where there's a need to get the full list of hidden block types, that becomes harder. The selector would need to filter the preferences down to only those with the 'hiddenBlockTypes' prefix. That might be performance intensive, given selectors are generally called more often than actions.
Testing Instructions
trunkopen the post editor and dismiss the welcome guide