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

Widget inserter: Clarify that the button toggles the inserter #33561

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

walbo
Copy link
Member

@walbo walbo commented Jul 19, 2021

Description

Follow-up on #29759
Add the same functionality to the inserter button on the widget screen and rename the label.

How has this been tested?

Locally by toggling the inserter sidebar.

Screenshots

widget-inserter

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@Mamaduka Mamaduka added [Feature] Widgets Screen The block-based screen that replaced widgets.php. [Package] Edit Widgets /packages/edit-widgets labels Jul 20, 2021
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks, @walbo!

@draganescu
Copy link
Contributor

👋🏻 how come we need to implement this in this specific editor? Isn't that application header common accross all editors?

@walbo
Copy link
Member Author

walbo commented Jul 20, 2021

👋🏻 how come we need to implement this in this specific editor? Isn't that application header common accross all editors?

Seems like the same functionality is implementet both in edit-post and edit-site package. As you say these should probably be aligned in a common component.

I can create a alternative PR later today that explores shared code.

@Mamaduka
Copy link
Member

I had a similar question about the different component a few months ago - #31597 (comment).

TL;DR - Sometimes duplication is better than bad early abstraction.

@Mamaduka Mamaduka merged commit dffade6 into WordPress:trunk Jul 20, 2021
@github-actions github-actions bot added this to the Gutenberg 11.2 milestone Jul 20, 2021
@walbo walbo deleted the widget-inserter-button branch July 20, 2021 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widgets Screen The block-based screen that replaced widgets.php. [Package] Edit Widgets /packages/edit-widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants