Add preferences modal to site editor #39485
Merged
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: +1.71 kB (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
tellthemachines
requested review from
talldan,
Mamaduka,
jasmussen and
kevin940726
8 tasks
Mamaduka
reviewed
Mar 17, 2022
| export default function EnableFeature( props ) { | ||
| const { featureName, ...remainingProps } = props; | ||
| const isChecked = useSelect( ( select ) => | ||
| select( preferencesStore ).get( 'core/edit-site', featureName ) |
There was a problem hiding this comment.
Suggested change
| select( preferencesStore ).get( 'core/edit-site', featureName ) | |
| ( select ) => | |
| select( preferencesStore ).get( 'core/edit-site', featureName ), | |
| [ featureName ] |
Missing dependency.
talldan
approved these changes
Mar 17, 2022
| export default function EnableFeature( props ) { | ||
| const { featureName, ...remainingProps } = props; | ||
| const isChecked = useSelect( ( select ) => | ||
| select( preferencesStore ).get( 'core/edit-site', featureName ) |
There was a problem hiding this comment.
I'd maybe just coerce this into a bool to be on the safe side 😄
Suggested change
| select( preferencesStore ).get( 'core/edit-site', featureName ) | |
| !! select( preferencesStore ).get( 'core/edit-site', featureName ) |
| } | ||
| return ( | ||
| <PreferencesModal closeModal={ toggleModal }> | ||
| <PreferencesModalTabs sections={ sections } /> |
There was a problem hiding this comment.
It does still feel a bit empty with one setting per tab. I wonder if right now an option could be to not add the tabs until there are more settings later on, and instead render the two PreferenceModalSections directly in the modal.
I don't know if the styles will work out of the box, but it might be possible to make some quick fixes there.
Could be good to get some design feedback on it though.
There was a problem hiding this comment.
I think tabs will fill up faster now that we've modal.
There was a problem hiding this comment.
Should be pretty easy to get the block manager in there as well now.
I think the component needs to be moved out of edit-post, but the rest of the implementation shouldn't be too difficult.
There was a problem hiding this comment.
Yeah, I added the second tab mainly thinking about the block manager which takes up a huge chunk of space 😄
Plus with #38317 there'll be at least one more item under Appearance soon.
Mamaduka
approved these changes
Mar 17, 2022
There was a problem hiding this comment.
This looks and works great. Thanks, @tellthemachines.
As you mentioned, we can progressively add new preferences and sections.
|
Nice, looks good! This is a good improvement. Just for posterity, post editor: Site editor:
Lots of drift between the two, which is entirely unrelated to this PR, but it's nice that the preferences modal now becomes unified between them! |
apeatling
approved these changes
Mar 17, 2022
There was a problem hiding this comment.
Looks good and works as expected when compared to the post editor.
@jasmussen Seeing this made me wonder if modals like this should contract to a smaller height if there's not enough content to fill it up? I guess the issue there would be changing tabs and the height of the modal changing?
|
Oh yeah absolutely, a whole iteration could be spent making modals have the right size, both intrinsicaly and across the board. Right now there are a bunch of conflicting and messy media queries and we could definitely be smarter there. I do think that without an explicit width attached, it'll collapse to the content, so yes that means it would shift between tabs as well. We encountered that with a recent Patterns explorere view where the window would flicker as the patterns loaded. It'd be nice if there was a smart way we could handle that, without a bunch of different min-heights. |
jostnes
pushed a commit
to jostnes/gutenberg
that referenced
this pull request
Mar 23, 2022
* Add preferences modal to site editor * Add spotlight feature * Add caret inside block setting * Improvements to enable feature selector.
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.
What?
This PR adds the Preferences modal to the site editor, with the aim of later filling in all the preferences we might find useful.
Why?
Split off from this comment. Tl,dr: in order to add more user preferences common to all editors, it's best to show them in a common UI.
Related: #31965
How?
Adds the recently abstracted reusable PreferencesModal from the interface package. For now, this adds only the "Spotlight mode" preference umder "General" and the "Contain text cursor inside block" preference under "Blocks". The plan is to progressively add all the other settings that make sense in the site editor, such as "Display button labels" (in progress: #38317) and the "Visible blocks" section. Suggestions welcome as to which other options we might add 🙂
Testing Instructions
Screenshots