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

Fix/wrong classes search block #26052

Merged

Conversation

MaggieCabrera
Copy link
Contributor

Description

  • Added missing width and height to the svg on the frontend
  • Added has-icon class to the button when it's an icon isntead of text on the frontend (this was missing compared to the editor)
  • Swapped the classes wp-block-search__icon-button and wp-block-search__text-button on the editor (they were correct on the frontend)

How has this been tested?

  • Create search blocks that have icon button on the different positions it can have it.
  • Create search blocks that have text button on the different positions too.
  • Check the frontend, the icon shouldn't be huge.
  • Check on the editor that when the block has an icon instead of text, the class wp-block-search__icon-button is applied. This doesn't have any visual impact, but it's needed for theming purposes.
  • Check on the editor that when the block has an text instead of icon, the class wp-block-search__text-button is applied. This doesn't have any visual impact, but it's needed for theming purposes.

Screenshots

Before:

Editor:

Screenshot 2020-10-13 at 10 50 01

Frontend:

Screenshot_2020-10-13 Search – gutenberg

After

Frontend:

Screenshot 2020-10-13 at 11 41 58

Types of changes

Class changes and added width/height to svg that was missing. My changes bring the frontend result more in line with what the editor shows.

Fixes #25902

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

@kirilzh kirilzh left a comment

Choose a reason for hiding this comment

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

Thank you for this PR!

Looks good to me!

@scruffian scruffian merged commit bd598df into WordPress:master Oct 14, 2020
@github-actions
Copy link

Congratulations on your first merged pull request, @MaggieCabrera! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 14, 2020
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Oct 14, 2020
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.

Search block: Missing/wrong classes when using icon instead of text in the button
4 participants