-
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
Add editor preference for default state of Link UI settings drawer #52321
Conversation
Size Change: +6.6 kB (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
I'll obviously fix the tests if folks agree this should land. |
settingsDrawerStatePreference: | ||
postEditorEnabled || siteEditorEnabled, | ||
}; |
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.
The return value should be determined based on which editor we are currently within. Else. setting from post editro will apply in Site Editor (and vice versa).
Any ideas or prior art on this. Maybe @talldan will know as he was the architect of the prefs store? 🙏
Nit but this preference feels like it does not belong in General > Appearance. Either one of General > Publishing, Blocks >Block Interactions feels better to me. |
Issues blocking this PR
|
@draganescu In all your work on DFM have you encountered a situation where you want to selectively get the preference for DFM from the relevant preference store depending on which editor you are in? |
The preferences are scoped per editor.
|
Yes but is there any prior art for getting the scoped preference based on your current editor? Update: I have a plan to pass the "scope" via the editor's settings in the block editor settings. Then it can be an optional thing. That should work. |
This feels like a very niche setting. My concern is that its another thing to maintain forever. Is there another setting we could make this a part of - like a "power use" kind of setting so we can group all these things under it? Is my concern about maintenance valid even? |
It does. Another idea would be to persist the "Advanced" open/closed state instead of a control to explicitly set it. |
Perhaps. Advanced editing controls could be an interesting area to explore. Not sure what else would be included. |
Another idea (I think) is to just save the state of the advanced section across all instances of the component - so once it's open in one place its open everywhere, until you close it... |
This PR would also fix the recently submitted #52489. |
@scruffian That's a nice idea, but should we not try to land this preference first and then come back to automated behaviours? |
I pushed a commit which shows an alternative route of providing the preference via the block editor settings. Now need to work out how to do the same as I've done for |
I'm not sure if we want to commit to a preference like this. Do you think we'd be able to remove it in the future? |
Flaky tests detected in 6bc1af6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5543933497
|
Yes. If the UI doesn't have "Advanced" then we can just remove it surely? |
You could do that if you wanted. I didn't want to over engineer anything at this stage. @WordPress/gutenberg-design what do we think about
|
Is there really a need to have the hidden option to open a link in a new tab, and to resolve this, have an option in the preferences panel? As it stands in previous versions of Gutenberg, and in the current version of WordPress, it's simple and works perfectly well. Why not leave it like that? Is it worth spending time and energy on this when there is so much in WordPress that could really be improved? These are just some of my thoughts... |
I agree that a preference to force the drawer open seems a little arbitrary. Looking at the original issue, the problem seems most prominent when you always want links to open in a new tab/window. So if we went down the preferences road, I wonder if it should be framed that way instead. IE allow the user to specify that links open in a new tab by default. All that said, persisting the "Advanced" open/closed state might be a simpler thing to try? |
@ricjcs Thanks for your feedback.
No - this is a good point. Not whilst there is only a single setting. There are better options (see what I suggest below in response to @jameskoster ).
It is an option that I'm proposing based on feedback left by other WordPress community users/contributors. If folks disagree (as they have done) then this is the ideal forum to voice objections and offer counter proposals.
Because (unfortunately) it doesn't (seem to) work really well. See the Issue linked to this PR.
Yes it is. There are lots of things contributors could/should work on and this is one of them. It's entirely valid to try and resolve this issue when a users has expressed frustration with how it behaves. This is particular so during this stage in the WP release process where major changes cannot be committed to the 6.3 release anyway. I would welcome any suggestions for alternatives to resolve the problem outlined in the linked Issue. For example, how do you see this working when (in the future) there are multiple settings in the "Advanced" drawer? @jameskoster Thanks for your thoughts. The problem is we need so many clicks to access the new tab (see linked Issue) and that's annoying. A good complimentary "fix" to this one would be to simply remove the link settings drawer altogether when there is only a single setting. @richtabor what do you think about that one? However in the future we plan to add more settings here so its short term and is not a full resolution to the problem expressed in the Issue. I'm happy to modify the PR to have a preference for default all links created using the LinkControl to be Appreciate the setting seems arbitrary but one could argue the same for The only alternative I can see if what @scruffian and others suggested, where once the drawer has been opened it toggles the preference to I welcome further discussion here. This is a valid problem which will affect many users and thus it needs a solution. Thanks all 🙇♂️ |
Hi @getdave, thanks for your reply and clarification, what you say makes sense.
In that case, couldn't the "Open in new tab" option, which is used by many users, be left out of the "Advanced" drawer? In the "Advanced" drawer would be the options that are perhaps less used. Thus, it would no longer be necessary to have an option that would allow the "Advanced" drawer to be opened by default, or persistence in open/closed state. I don't know if in the future there will be many options in the "Advanced" drawer, if there will be a good user experience to keep the drawer always open. |
This seems like a good way forward. |
In general I've been told we don't want to prioritise this setting because:
@scruffian As few of us agree that having a concrete preference is a good idea, I'm happy to try this approach. Note that means it's not going to make 6.3 however. |
I am also on the side of simply persisting the satte of the drawer - a soft of magic preference, instead of yet another toggle. |
I think that if these future options had the same level of importance, they would already exist.
I think that by not being turned on by default already fulfills this purpose. But, if "Open in new tab" is really going to stay inside the "Advanced" drawer, then persistence of the drawer state seems to be the best way. |
I'm not opposed to this tbh. |
Closing in favour of #52799 which seems simpler and "just works" even if it might be a bit "magic". I think on balance it's a better solution. |
What?
Fixes regression where "optimising" the Link UI for 80% of users has made it worse for 20% who want to use "Open in new tab".
Sets an editor preference for the default open/closed state of the Link UI's settings drawer.
Closes #52216
Why?
Whilst we don't want to encourage usage of open in new tab it's still a use case for some. By adding a preference users can opt-in to always displaying the settings drawer which improves the UX for them.
How?
Add preference and use as default state for link UI.
Question: is there a way to get the appropriate preference store depending on which editor I'm in?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast