Skip to content
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

Cross-editor settings #24370

Closed
tellthemachines opened this issue Aug 5, 2020 · 15 comments
Closed

Cross-editor settings #24370

tellthemachines opened this issue Aug 5, 2020 · 15 comments
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") Needs Design Needs design efforts. [Package] Interface /packages/interface

Comments

@tellthemachines
Copy link
Contributor

Is your feature request related to a problem? Please describe.

With the advent of the site editor, we'll have two very similar editors (post and site editor), each with their own settings for UI customization: full screen, spotlight mode, etc. Some of these may make sense to set on a per-editor basis, but others (such as the icon labels setting in progress in #24304) should be configurable for both editors at once.

What might be the best way of doing this? The wp-admin "Settings" come to mind, but it might be confusing to have a setting there that only applies to the new editors.

@noahtallen
Copy link
Member

I definitely agree about this in principle. I think there is still some discussion (cc @youknowriad) about how much code needs to be shared between the two editors. For example, maybe the site editor will eventually just be a "mode" of the general post editor. That might make it easier to share certain settings.

@youknowriad
Copy link
Contributor

Yes, we discussed this in the past. Basically we have three options:

1- Copy/paste code between edit-site, edit-post, edit-widgets...
2- Create shared components and shared stores (avoid these as much as possible though, it should be done for generic things like notices, shortcuts..;) and use them in the different packages. The difficulty here lays in finding the right abstraction and the right place for the abstraction, it's very hard to get right.
3- Call edit-post in edit-site with options. Doesn't work for edit-widgets and depending on how edit-post and edit-site diverge, it can be very tricky too.

I think ultimately, the second approach is probably the best but I also think that we should not be afraid of doing 1 as a start. Copy/pasting is better than a bad abstraction (see this talk for some literature https://overreacted.io/the-wet-codebase/ ). Over time abstractions become clearer.

It's important to keep in my mind that abstractions are in general public APIs we have to maintain forever.

@youknowriad
Copy link
Contributor

There's also a related question about the shared settings. Some settings should be shared but others shouldn't. I mean if I enable fullscreen for edit-post, I'm not sure I want fullscreen for edit-widgets and edit-site.

@noahtallen
Copy link
Member

There are also some confusing subdivisions among settings. For example, we have "preferences" and we have "features" and we have "settings", and sometimes things that might feel like settings (like deviceType from the preview, or navigationMode from the mode switcher) aren't stored in any of the above subdivisions.

@tellthemachines
Copy link
Contributor Author

Some settings should be shared but others shouldn't. I mean if I enable fullscreen for edit-post, I'm not sure I want fullscreen for edit-widgets and edit-site.

Yup, that makes sense. But in other cases such as the button labels setting, I think it should be consistent across both editors. My main concern is that users should be able to only set this once. Ideally, we should use site settings and make things work for the whole of wp-admin, but, depending on the setting, that might be harder to wrangle as it can involve lots of changes across both codebases.

On the other hand, specifically for the button labels issue, there don't seem to too many icon-only buttons across wp-admin, outside of the editors, so perhaps a global admin setting would be viable in that case?

@talldan
Copy link
Contributor

talldan commented Aug 11, 2021

Related to this I started refactoring 'preferences' to a shared place in #33774

This happened after trying this for the widget editors:

1- Copy/paste code between edit-site, edit-post, edit-widgets...

It turned out to be not very maintainable, so it moved onto this:

Create shared components and shared stores (avoid these as much as possible though, it should be done for generic things like notices, shortcuts..;) and use them in the different packages. The difficulty here lays in finding the right abstraction and the right place for the abstraction, it's very hard to get right.

@talldan talldan added Needs Design Needs design efforts. [Package] Interface /packages/interface labels Aug 26, 2021
@talldan
Copy link
Contributor

talldan commented Aug 26, 2021

While this is now possible to achieve, I think there are still some challenges.

There are now three separate editors in WordPress core - the post editor, and the two widgets editors. Each of these stores preferences separately, so there's now a situation where a user might have conflicting values for the same preference (e.g. top toolbar enabled in one editor but not in the others). There's a challenge in migrating those to a single global preference.

Then there are various ways to store a global preference:

  • Preferences could be singleton values, they use a shared key in the interface store and changing them in one place changes them in all places
  • Preferences could be made up of multiple values (stored under a separate key for each editor), but the user is able to change them all to one value in a single action (e.g. for example using a global setting in the WordPress settings menu)
  • A global preference could alternatively act more like a default value, but the user can still override it in an individual editor

There's also a UX aspect if some settings are global and affect all editors, but some only affect the one editor.

@tellthemachines
Copy link
Contributor Author

A global preference could alternatively act more like a default value, but the user can still override it in an individual editor

This sounds like the less intrusive way of doing it, as implementing it won't change current user preferences. It's also how I would expect such a thing to work, if we had both global and local settings 😄

@youknowriad
Copy link
Contributor

It sounds as if preferences could be its own package and be a dependency of all other screens with the possibility to have both global preferences and specific preferences made accessible.

@talldan
Copy link
Contributor

talldan commented Sep 1, 2021

I've already mostly migrated them to interface. Would have been good to get some feedback on a preferences package earlier - #31965.

@youknowriad
Copy link
Contributor

interface is becoming kind of bloated package, it shouldn't be seen as a package to contain anything that can be shared across screens. Its original purpose was to help build the interface/UI of the editor screens. Maybe its name is confusing and encourages for a "utils" kind of package

@talldan
Copy link
Contributor

talldan commented Sep 1, 2021

That's fine - the timing is bad because I asked for feedback on the migration to interface a few weeks ago and got approved PRs. The code is most of the way towards migrated to the interface package.

Refactoring all that code again to a new package is something I don't really have time to do, so I'll remove my assignment from that. Happy for another willing contributor to pick it up.

@youknowriad
Copy link
Contributor

I'm sorry I missed that ping :( it's becoming very hard to follow everything and I understand that everyone has their own assignments and tasks to handle.

It's just an idea for now 🤷‍♂️

@tellthemachines
Copy link
Contributor Author

These preferences are all related to how the interface is presented, or to how it presents the content. Doesn't it make sense for them to be in the interface package? If we always use them together with interface components, there's not much to gain in splitting them out to another package.

@annezazu annezazu added [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") and removed [Feature] Full Site Editing labels Jul 24, 2023
@youknowriad
Copy link
Contributor

I think there has been a lot of iterations on this frontend. The current state is not perfect but good enough for now. Closing this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") Needs Design Needs design efforts. [Package] Interface /packages/interface
Projects
None yet
Development

No branches or pull requests

5 participants