Skip to content

ToolsPanel: Prevent calling deselect when panel remounts#45673

Merged
andrewserong merged 3 commits intotrunkfrom
fix/block-support-resets-on-block-drop
Nov 10, 2022
Merged

ToolsPanel: Prevent calling deselect when panel remounts#45673
andrewserong merged 3 commits intotrunkfrom
fix/block-support-resets-on-block-drop

Conversation

@aaronrobertshaw
Copy link
Copy Markdown
Contributor

Fixes: #43491

What?

Prevents controls within a ToolsPanel from erroneously calling their onDeselect callback and resetting the control's related block attribute when the ToolsPanel gets remounted, e.g. when a block is dropped in a new location.

Why?

Losing customizations you've made is bad.

How?

If the ToolsPanel has been remounted, its internal state for menu items will also be reset. Items added to the panel via SlotFill will persist while the panel gets remounted. This leads to their derived state being out of sync with the context value provided by the panel. This PR checks whether the item is currently registered with the panel and, if it isn't, skips any calls to onSelect or onDeselect callbacks.

Testing Instructions

  1. Before checking out this PR, edit a post, add multiple paragraph blocks, and move one paragraph into a group block.
  2. Select the paragraph within the group block and apply custom typography styles for all typography options
  3. Save the post
  4. Drag the paragraph block to a new position outside of the group and notice that several typography customizations are lost e.g. Font family.
  5. Checkout this PR and reload the editor
  6. Drag the paragraph block out of the group block again and note that all typography customizations are maintained

Screenshots or screencast

Screen.Recording.2022-11-09.at.3.52.09.pm.mp4

@codesandbox
Copy link
Copy Markdown

codesandbox bot commented Nov 9, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@aaronrobertshaw aaronrobertshaw added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi labels Nov 9, 2022
@dsas
Copy link
Copy Markdown
Contributor

dsas commented Nov 9, 2022

This tests nicely for me 👍

Copy link
Copy Markdown
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Nice one, thanks for fixing this up @aaronrobertshaw!

✅ Confirmed the issue on trunk
✅ With this PR applied, moving a paragraph block with all typography settings in use out of a Group block and to elsewhere in the document now preserves all the settings in use

LGTM! ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Various typography changes reset after moving paragraph block out of quote block

3 participants