-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Make the search block editor match the frontend #30176
Conversation
// This rule only makes sense for UI not blocks. | ||
// eslint-disable-next-line react/forbid-elements | ||
<button type="submit" className="wp-block-search__button"> | ||
<RichText |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems there's something that prevents RichText from working as a "button". I've used a small hack here to make it work and still keep a similar markup to the frontend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What prevents it from working? Do you get any errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just stuck, like you can't edit it. Maybe "contenteditable" doesn't work on buttons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's interesting. I'll try this out in a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work: https://codepen.io/iseulde/pen/vYgOpVw
Maybe something is wrong in RichText.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason the selection change event is not firing in this context. It is in codepen though. Strange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems something is the block editor is causing the issue. I insert <button contenteditable>test</button>
without rich text in a block and it also fails to set selection. The same line on an empty web page works fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ellatrix after the removal of revert: all, I have two bugs:
- In safari, backspace removes the block and not a character in the text
- in chrome, I can't type an empty space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yes, Space will conflict with "pressing" the button.
I'll have a look at both issues. In the meantime, feel free to nest it in a span and we can adjust later.
Size Change: -37 B (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
textarea, | ||
button { | ||
font-family: system-ui; | ||
all: revert; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for the public record :) all: revert;
is also reverting -webkit-user-modify
to read-only
preventing contenteditable
from working.
I agree with this but I don't think it should be done in these PRs because it has implications and impact on themes which can be higher and it should be done very carefully. For now, the purpose is just to make the backend look like the frontend without impacting the frontend. I'll fix 2021 |
bb005ae
to
2385eff
Compare
I really appreciate this PR. I just ran into issues where the use of editor components adds some extra classes and therefore specificity to the rules, which means it's really really tricky for a theme to style the form widgets of this one in an elegant way. It's specifically the and on the frontend: And here's after, in the editor: and on the frontend: In other words, to me this solves a problem of the style inheritance very nicely. |
Yes, this is a good PR. Unfortunately, the reset used here conflicts with placeholder buttons. It's the same issue we have with themes styling |
Closing this as still blocked by CSS bleed in placeholder components. It's unclear how we'll solve this. |
Related #29976
This block also relies on "input" and "button" element which in wp-admin has some opinionated styles, this PR resets those styles in the editor canvas.
This PR needs to be tested properly (specially the editor UI to ensure the select reset is not affecting anything that shouldn't).
At the moment, this PR impact placeholders in other blocks because these resets have higher specificity than the UI components used in placeholders (Button, TextControl...). We have three options:
Use a similar technique as in Editor Styles: Add a transform for button rules #29229 and add:not(.components-button)
and similar to the resets.