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

WIP: Only send one class name to HTML API's add_class() #54873

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Sep 27, 2023

Previously the search block has been passing multiple escaped CSS class names to add_class() when the API only expects a single one.

For example, add_class( 'one-class another-class' ).

This has mostly worked, but exposes a bug and ideally the code should match the expectations of the API. In the future, add_class() will reject these calls.

In this patch we're updating the search block code to only pass in a single class name. This also involves some refactoring to return class names from helper functions in a structured form. By doing this additional work we can avoid joining a string just to split it apart immediately aftwards. Because the HTML API handled escaping, extraneous calls to esc_attr have been removed.

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

@dmsnell dmsnell added [Type] Enhancement A suggestion for improvement. [Type] Code Quality Issues or PRs that relate to code quality [Block] Search Affects the Search Block - used to display a search field labels Sep 27, 2023
@github-actions
Copy link

Warning: Type of PR label error

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] Accessibility (a11y), [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] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
  • Labels found: [Type] Enhancement, [Type] Code Quality, [Block] Search.

Read more about Type labels in Gutenberg.

@dmsnell dmsnell removed the [Type] Enhancement A suggestion for improvement. label Sep 27, 2023
@dmsnell dmsnell force-pushed the html-api-refactor/block-library-search--only-one-class-name branch 2 times, most recently from 2a3eb52 to 9217133 Compare October 15, 2023 00:28
Previously the search block has been passing multiple escaped CSS
class names to `add_class()` when the API only expects a single one.

For example, `add_class( 'one-class another-class' )`.

This has mostly worked, but exposes a bug and ideally the
code should match the expectations of the API. In the future,
`add_class()` will reject these calls.

In this patch we're updating the search block code to only pass
in a single class name. This also involves some refactoring to
return class names from helper functions in a structured form.
By doing this additional work we can avoid joining a string just
to split it apart immediately aftwards. Because the HTML API
handled escaping, extraneous calls to `esc_attr` have been removed.
@dmsnell dmsnell force-pushed the html-api-refactor/block-library-search--only-one-class-name branch from 9217133 to f4cd8ea Compare October 31, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Search Affects the Search Block - used to display a search field [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant