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.

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

Fix sticking “Reset” option in ToolsPanel #60621

Merged
merged 7 commits into from
May 9, 2024
+26 −14
Merged

Fix sticking “Reset” option in ToolsPanel #60621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
643cdc2
Fix sticking “Reset” option in `ToolsPanel`
stokesman Apr 9, 2024
87adceb
Tweak block-editor’s `DimensionTool` to avoid updating tests`
stokesman Apr 10, 2024
7a960f1
Revert "Tweak block-editor’s `DimensionTool` to avoid updating tests`"
stokesman Apr 11, 2024
753a8bf
Avoid calling `onDeselect` if item’s value is not set
stokesman Apr 11, 2024
48a5792
Skip flagging item customization for optional items without value
stokesman Apr 17, 2024
fa554e4
Add changelog entry
stokesman May 9, 2024
0b0d8c7
Format
stokesman May 9, 2024
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
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
- `FormTokenField`: Hide label when not defined ([#61336](https://github.com/WordPress/gutenberg/pull/61336)).
- Upgraded the @types/react and @types/react-dom packages ([#60796](https://github.com/WordPress/gutenberg/pull/60796)).

### Bug Fix

- `ToolsPanel`: Fix sticking “Reset” option ([#60621](https://github.com/WordPress/gutenberg/pull/60621)).

## 27.5.0 (2024-05-02)

### Enhancements
Expand Down
20 changes: 12 additions & 8 deletions packages/components/src/tools-panel/tools-panel-item/hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,17 +125,21 @@ export function useToolsPanelItem(
const isRegistered = menuItems?.[ menuGroup ]?.[ label ] !== undefined;

const isValueSet = hasValue();
const wasValueSet = usePrevious( isValueSet );
const newValueSet = isValueSet && ! wasValueSet;

// Notify the panel when an item's value has been set.
// Notify the panel when an item's value has changed except for optional
// items without value because the item should not cause itself to hide.
useEffect( () => {
if ( ! newValueSet ) {
if ( ! isShownByDefault && ! isValueSet ) {
return;
}

flagItemCustomization( label, menuGroup );
}, [ newValueSet, menuGroup, label, flagItemCustomization ] );
flagItemCustomization( isValueSet, label, menuGroup );
}, [
isValueSet,
menuGroup,
label,
flagItemCustomization,
isShownByDefault,
] );

// Determine if the panel item's corresponding menu is being toggled and
// trigger appropriate callback if it is.
Expand All @@ -151,7 +155,7 @@ export function useToolsPanelItem(
onSelect?.();
}

if ( ! isMenuItemChecked && wasMenuItemChecked ) {
if ( ! isMenuItemChecked && isValueSet && wasMenuItemChecked ) {
onDeselect?.();
}
Comment on lines -154 to 160
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The effect that triggers this runs more often now that flagItemCustomization get called when removing a value. That was causing onDeselect to be called multiple times and this change fixed that.

As an aside, I experimented with a little refactor to not call these (onSelect/onDeselect) from an effect and it seemed to work well and simplifies things a bit. I wanted to keep changes minimal so I've left it out but if that sounds good I'd happily make a PR for it.

}, [
Expand Down
15 changes: 9 additions & 6 deletions packages/components/src/tools-panel/tools-panel/hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,18 +205,21 @@ export function useToolsPanel(
} );
}, [ panelItems, setMenuItems, menuItemOrder ] );

// Force a menu item to be checked.
// This is intended for use with default panel items. They are displayed
// separately to optional items and have different display states,
// we need to update that when their value is customized.
// Updates the status of the panel’s menu items. For default items the
// value represents whether it differs from the default and for optional
// items whether the item is shown.
const flagItemCustomization = useCallback(
Comment on lines -208 to 211
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment seemed out of date because the callback is already being used from optional panel items and now, of course, it’s being used to also “remove” a customization flag.

( label: string, group: ToolsPanelMenuItemKey = 'default' ) => {
(
value: boolean,
label: string,
group: ToolsPanelMenuItemKey = 'default'
) => {
setMenuItems( ( items ) => {
const newState = {
...items,
[ group ]: {
...items[ group ],
[ label ]: true,
[ label ]: value,
},
};
return newState;
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/tools-panel/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export type ToolsPanelContext = {
registerResetAllFilter: ( filter: ResetAllFilter ) => void;
deregisterResetAllFilter: ( filter: ResetAllFilter ) => void;
flagItemCustomization: (
value: boolean,
label: string,
group?: ToolsPanelMenuItemKey
) => void;
Expand Down
Loading