Skip to content

Refactor editor 'feature' preferences to interface package #33774

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.

Sign up for GitHub

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

Merged
talldan merged 35 commits into from
Aug 18, 2021
Merged

Refactor editor 'feature' preferences to interface package #33774

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
3827a18
Add basic store state for preferences
talldan May 20, 2021
c52bf90
Add more menu component
talldan May 20, 2021
23f7920
Persist preferences
talldan May 20, 2021
f9fb645
Fix translator comment
talldan May 20, 2021
2a3073d
Add a few refinements to the dropdown menu component
talldan May 20, 2021
e19f5e2
Add dropdown menu styles
talldan May 20, 2021
5a27418
Use more menu for edit widgets
talldan May 20, 2021
0b67393
Update edit widgets to use feature toggles
talldan May 20, 2021
8b664e2
Remove some dead code
talldan May 20, 2021
d5497d1
Allow setting default feature values
talldan May 20, 2021
6329f05
Update some newer preferences after rebase
talldan Jul 30, 2021
ed0bba4
Remove invalid tests
talldan Jul 30, 2021
2891d29
Update package.json
talldan Jul 30, 2021
1704f95
Migrate editor preferences to edit-widgets
talldan Aug 5, 2021
b98850a
Update e2e test code for disabling the welcome guide
talldan Aug 5, 2021
6a2e2c7
Move preference defaults to a separate reducer to avoid persistence
talldan Aug 5, 2021
51442e1
Fix - export defaults reducer
talldan Aug 5, 2021
5f4e4c0
Add component docs and missing selector doc
talldan Aug 5, 2021
6d50c38
Add main readme docs
talldan Aug 5, 2021
9dd1b0e
Addthe example to the component docs as well
talldan Aug 5, 2021
a6e40c8
Update changelogs
talldan Aug 5, 2021
fcdbb3d
Add tests for selector
talldan Aug 5, 2021
0fcaf7c
Ensure toggling takes into account the default value
talldan Aug 9, 2021
f44f392
Rename MoreMenuDropdown to OptionsMenuDropdown
talldan Aug 16, 2021
4e6b560
Rename MoreMenuFeatureToggle to OptionsMenuFeatureTogle
talldan Aug 16, 2021
4b48a36
Rename OptionsMenuDropdown to OptionsMenu
talldan Aug 16, 2021
0b2b8c0
Fix style import
talldan Aug 16, 2021
b0f879b
Fix migration handling for multiple stores
talldan Aug 16, 2021
ef7da49
Update docs for migration function
talldan Aug 16, 2021
028565d
Fix store name
talldan Aug 16, 2021
e9c80c4
Revert "Fix style import"
talldan Aug 18, 2021
9931b6d
Revert "Rename OptionsMenuDropdown to OptionsMenu"
talldan Aug 18, 2021
b9e6fd3
Revert "Rename MoreMenuFeatureToggle to OptionsMenuFeatureTogle"
talldan Aug 18, 2021
5067fc2
Revert "Rename MoreMenuDropdown to OptionsMenuDropdown"
talldan Aug 18, 2021
9f58d3a
Remove setFeatureDefaults as an API
talldan Aug 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename MoreMenuFeatureToggle to OptionsMenuFeatureTogle
  • Loading branch information
@talldan
talldan committed Aug 18, 2021
commit 4e6b560e025a215f69b864df43f4bcec72c6ccb4
12 changes: 6 additions & 6 deletions packages/edit-widgets/src/components/more-menu/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { __, _x } from '@wordpress/i18n';
import { external } from '@wordpress/icons';
import {
OptionsMenuDropdown,
MoreMenuFeatureToggle,
OptionsMenuFeatureToggle,
} from '@wordpress/interface';
import { displayShortcut } from '@wordpress/keycodes';
import { useShortcut } from '@wordpress/keyboard-shortcuts';
Expand Down Expand Up @@ -43,7 +43,7 @@ export default function MoreMenu() {
<>
{ isLargeViewport && (
<MenuGroup label={ _x( 'View', 'noun' ) }>
<MoreMenuFeatureToggle
<OptionsMenuFeatureToggle
scope="core/edit-widgets"
Copy link
Contributor Author

@talldan talldan Jul 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having to define core/edit-widgets everywhere gets a bit repetitive. An option could be a ScopeProvider as part of the interface package.

Copy link
Member

@gziolo gziolo Jul 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A local component might help, too:

const MoreMenuWidgetsFeatureToggle = ( props ) => <MoreMenuFeatureToggle { ...props } scope="core/edit-widgets />;

I think that it's how this issue is mitigated in the edit post package. It had some backward compatiblity considerations, so we can explore other ideas, too.

Copy link
Member

@kevin940726 kevin940726 Aug 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of using a Provider. We can embed the provider inside <InterfaceSkeleton> seamlessly unless there're needs to use the components outside the tree.

An alternative could be to use hook-like API to re-implement this.

// edit-widgets/src/components/preference.js
const Preference = createPreference( 'core/edit-widgets', defaultPreferences );

export const PreferenceProvider = Preference.Provider; // The provider component
export const getPreference = Preference.get; // Getter function to get the preference data
export const setPreference = Preference.set; // Setter function to set the preference data
export const usePreference = Preference.usePreference; // A hook to react to changes and be used in components

So that all descendent components can use the hook to automatically get the preference data without having to specify scope.

A downside of this API is that it's not an idiomatic @wordpress/data-style selector we're used to, so it might be harder for third-party developers to hook into.

I'm in favor of the current approach, just want to throw out some alternative approaches to see if it can bring up some ideas from others.

Copy link
Contributor Author

@talldan talldan Aug 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of using a Provider. We can embed the provider inside seamlessly unless there're needs to use the components outside the tree.

Yeah, this is exactly what I was thinking. It would still be possible for scope to be overridden if a prop is provided to a context using component, but I wouldn't see that being a common use case.

There are other components within the interface package that use scope, so I'd have to double-check how they work, but I can work on a separate proof of concept.

I also think it's worth seeing how this will work for editors that have more settings, edit/widgets isn't really a great example of this being a challenge, it may be that @gziolo's suggestion is easier and makes more sense in the post editor.

One point on that is that context won't help for the selectors or actions, the solutions there would be a custom react hook, or alternatively registry selectors / generator actions. There's existing evidence of that here in the edit-post package:

gutenberg/packages/edit-post/src/store/actions.js

Lines 31 to 51 in 5a610d4

export function* openGeneralSidebar( name ) {
yield controls.dispatch(
interfaceStore.name,
'enableComplementaryArea',
editPostStore.name,
name
);
}
/**
* Returns an action object signalling that the user closed the sidebar.
*
* @yield {Object} Action object.
*/
export function* closeGeneralSidebar() {
yield controls.dispatch(
interfaceStore.name,
'disableComplementaryArea',
editPostStore.name
);
}

Copy link
Contributor

@tellthemachines tellthemachines Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If some of the options are shared across editors, we'll be using more than one scope here. Defining the scope manually might the best way to go until we've got this working everywhere.

Copy link
Contributor Author

@talldan talldan Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd still be ok with any of the approaches. It could be a separate component that doesn't use the context (GlobalFeatureToggle), or a prop that overrides the scope from context.

feature="fixedToolbar"
label={ __( 'Top toolbar' ) }
Expand All @@ -68,7 +68,7 @@ export default function MoreMenu() {
>
{ __( 'Keyboard shortcuts' ) }
</MenuItem>
<MoreMenuFeatureToggle
<OptionsMenuFeatureToggle
scope="core/edit-widgets"
feature="welcomeGuide"
label={ __( 'Welcome Guide' ) }
Expand All @@ -92,7 +92,7 @@ export default function MoreMenu() {
</MenuItem>
</MenuGroup>
<MenuGroup label={ __( 'Preferences' ) }>
<MoreMenuFeatureToggle
<OptionsMenuFeatureToggle
scope="core/edit-widgets"
feature="keepCaretInsideBlock"
label={ __(
Expand All @@ -108,7 +108,7 @@ export default function MoreMenu() {
'Contain text cursor inside block deactivated'
) }
/>
<MoreMenuFeatureToggle
<OptionsMenuFeatureToggle
scope="core/edit-widgets"
feature="themeStyles"
info={ __(
Expand All @@ -117,7 +117,7 @@ export default function MoreMenu() {
label={ __( 'Use theme styles' ) }
/>
{ isLargeViewport && (
<MoreMenuFeatureToggle
<OptionsMenuFeatureToggle
scope="core/edit-widgets"
feature="showBlockBreadcrumbs"
label={ __( 'Display block breadcrumbs' ) }
Expand Down
2 changes: 1 addition & 1 deletion packages/interface/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

### New Feature

- Add support for editor 'feature' preferences. Adds an `isFeatureActive` selector, a `toggleFeature` action, a `setFeatureDefaults` API function, a `OptionsMenuDropdown` component, and a `MoreMenuFeatureToggle` component. ([#33774](https://github.com/WordPress/gutenberg/pull/33774)).
- Add support for editor 'feature' preferences. Adds an `isFeatureActive` selector, a `toggleFeature` action, a `setFeatureDefaults` API function, a `OptionsMenuDropdown` component, and a `OptionsMenuFeatureToggle` component. ([#33774](https://github.com/WordPress/gutenberg/pull/33774)).

## 4.0.0 (2021-07-29)

Expand Down
4 changes: 2 additions & 2 deletions packages/interface/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,15 @@ wp.data.dispatch( 'core/interface' ).toggleFeature( 'namespace/editor-or-plugin-
wp.data.select( 'core/interface' ).isFeatureActive( 'namespace/editor-or-plugin-name', 'myFeatureName' ); // false
```

The `OptionsMenuDropdown` and `MoreMenuFeatureToggle` components help to implement an editor menu for changing preferences and feature values.
The `OptionsMenuDropdown` and `OptionsMenuFeatureToggle` components help to implement an editor menu for changing preferences and feature values.

```jsx
function MyEditorMenu() {
return (
<OptionsMenuDropdown>
{ () => (
<MenuGroup label={ __( 'Features' ) }>
<MoreMenuFeatureToggle
<OptionsMenuFeatureToggle
scope="namespace/editor-or-plugin-name"
feature="myFeatureName"
label={ __( 'My feature' ) }
Expand Down
2 changes: 1 addition & 1 deletion packages/interface/src/components/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ export { default as FullscreenMode } from './fullscreen-mode';
export { default as InterfaceSkeleton } from './interface-skeleton';
export { default as PinnedItems } from './pinned-items';
export { default as OptionsMenuDropdown } from './options-menu-dropdown';
export { default as MoreMenuFeatureToggle } from './more-menu-feature-toggle';
export { default as OptionsMenuFeatureToggle } from './options-menu-feature-toggle';
export { default as ActionItem } from './action-item';
4 changes: 2 additions & 2 deletions packages/interface/src/components/options-menu-dropdown/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

This component implements a `DropdownMenu` component from the `@wordpress/components` package.

See also the `MoreMenuFeatureToggle` component in the `@wordpress/interface` package, which provides an easy way to implement a feature toggle as a child of this component. Use with the `MenuGroup`, `MenuItem`, `MenuItemsChoice` components from the `@wordpress/components` package to implement more advanced behaviors.
See also the `OptionsMenuFeatureToggle` component in the `@wordpress/interface` package, which provides an easy way to implement a feature toggle as a child of this component. Use with the `MenuGroup`, `MenuItem`, `MenuItemsChoice` components from the `@wordpress/components` package to implement more advanced behaviors.

Note that just like the `DropdownMenu` component, this component accepts a render callback, which child elements should be returned from.

Expand All @@ -20,7 +20,7 @@ function MyEditorMenu() {
<OptionsMenuDropdown>
{ () => (
<MenuGroup label={ __( 'Features' ) }>
<MoreMenuFeatureToggle
<OptionsMenuFeatureToggle
scope="namespace/editor-or-plugin-name"
feature="myFeatureName"
label={ __( 'My feature' ) }
Expand Down
0 ...onents/more-menu-feature-toggle/README.md → ...nts/options-menu-feature-toggle/README.md
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion ...ponents/more-menu-feature-toggle/index.js → ...ents/options-menu-feature-toggle/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { speak } from '@wordpress/a11y';
*/
import { store as interfaceWidgetsStore } from '../../store';

export default function MoreMenuFeatureToggle( {
export default function OptionsMenuFeatureToggle( {
scope,
label,
info,
Expand Down