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

ToolsPanel: Prevent calling deselect when panel remounts #45673

Merged
merged 3 commits into from
Nov 10, 2022

Conversation

aaronrobertshaw
Copy link
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

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
Contributor

dsas commented Nov 9, 2022

This tests nicely for me 👍

Copy link
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