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

Add InserterPanel around block directory results. #21748

Conversation

StevenDufresne
Copy link
Contributor

Description

This PR addresses #21746 using the InserterPanel component.

How has this been tested?

Manual test by turning on Block Directory Experiment and searching for Boxer.

Screenshots

With Blocks:

Screenshot

Without Blocks:

Screenshot

Types of changes

This PR reestablishes the padding around the container but introduces 1 change. By using the InserterPanel component, a title had to be provided. I believe this helps in creating a consistent experiment but the copy should be scrutinized: Search Results.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • 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.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

From the first screenshot, I think maybe we should add a bit of padding between the panel title and the text "no blocks ..."

Change looks good though.

@StevenDufresne
Copy link
Contributor Author

@youknowriad Cool. I update the padding/margin. We were forcefully removing it.

Removed padding

@youknowriad
Copy link
Contributor

Looks great.

@youknowriad youknowriad merged commit bb07685 into WordPress:master Apr 23, 2020
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 23, 2020
@youknowriad
Copy link
Contributor

Just noticed that you've been using a fork of Gutenberg, you can use branches if you like as you're a member of the repo.

@StevenDufresne
Copy link
Contributor Author

Cool!

Thanks for the heads up!

@StevenDufresne
Copy link
Contributor Author

I kinda like the fork model because I don't need to worry about deleting branches. I can just kill my fork and move on.

@youknowriad
Copy link
Contributor

From the reviewer's perspective, using a branch is easier (we can checkout it more easily), that said, if you prefer using a fork, that's not a reason to change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants