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

Enhance: Added close on outside click flag & updated save post panel #67327

Conversation

Mayank-Tripathi32
Copy link
Contributor

Attempt to resolve #67313

What?

This PR adds a flag to the useDialog component to disable the behavior of closing the dialog when clicking outside of it. It also updates the relevant save_post code to prevent closing the dialog when clicked in specific areas.

Why?

The issue occurs when clicking on the 'padding area' of the Entity saved modal dialog, causing the dialog to close unexpectedly.

Animated GIF to illustrate:
image

How?

A closeOnOutsideClick flag was added to the useDialog component. By default, this behavior is set to true, meaning the dialog will close when clicking outside. The save_post file has been updated to disable this behavior for the issue mentioned above.

Testing Instructions

  • Go to the Site editor.
  • Make a simple change to a global style, e.g. change a color.
  • Reopen the Navigation panel by clicking 'Open Navigation' at the top left of the screen.
  • Click any item in the navigation other than 'Styles' e.g. click 'Pages' or 'Templates'.
  • At this point, a blue button at the bottom of the Navigation panel appears, labeled 'Review 1 change...'

image

  • Click the button: the modal dialog to save entities opens.
  • Click the padding area of the modal dialog.
  • Observe the modal dialog closes.
  • Alternatively preview a block theme from the WP admin > Appearance > any block theme > Live Preview.
  • Click the blue button labeled 'Activate {theme name}' at the bottom of hte Navigation panel: the modal dialog to save entities opens.
  • Click the padding area of the modal dialog.
  • Observe the modal dialog closes.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 26, 2024
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Mayank-Tripathi32! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@Mayank-Tripathi32 Mayank-Tripathi32 marked this pull request as ready for review November 26, 2024 18:27
Copy link

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core, Gutenberg Plugin.
  • Labels found: First-time Contributor.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

1 similar comment
Copy link

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core, Gutenberg Plugin.
  • Labels found: First-time Contributor.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

Copy link

github-actions bot commented Nov 26, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mayank-Tripathi32 <mayanktripathi32@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@youknowriad
Copy link
Contributor

Thanks for attempting a fix. That said, I think the issue needs to be solved in a different way.

The EntitiesSavedState component is shared between the "editor" package's panel and the modal in the "site editor". That said what's being shared is not the correct thing.

You can see that EntitiesSavedStatesExtensible is used in both and that it includes useDialog hook. I think the useDialog hook and the whole dialog behavior needs to be moved outside the EntitiesSavedStatesExtensible into a dedicated wrapper that is only used in the "editor" package. That way for the site editor the dialog behavior is implemented by the modal itself while the editor package has its dedicated dialog.

@Mayank-Tripathi32
Copy link
Contributor Author

@youknowriad

Thank you for the clarification. Would it be all right for me to work on implementing the wrapper? I believe it should be straightforward to adjust the behavior, and I can open a new PR with the required changes.

@youknowriad
Copy link
Contributor

Sure. Feel free to give it a try. Thanks for your help.

@Mayank-Tripathi32
Copy link
Contributor Author

Moved to #67352

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Entity saved modal dialog closes unexpectedly
2 participants