Underline text when cmd + u is pressed #13117
Conversation
jorgefilipecosta
added
the
[Type] Bug
|
+1 to merging this! Big gotcha (ran into it myself while writing this morning). |
| character="u" | ||
| onUse={ onToggle } | ||
| /> | ||
| <RichTextToolbarButton |
There was a problem hiding this comment.
Does this add a toolbar button or is this component just required to make the shortcut work?
If this isn't visible it's worth noting that in a comment; it's kinda confusing. (If it is visible, didn't we decided awhile back not to include underlines in the toolbar? According to #7162 the lack of a button was intentional.)
There was a problem hiding this comment.
No the button is not required. It should be removed.
There was a problem hiding this comment.
Hi @tofumatt and @iseulde thank you for your reviews.
I am adding the button here button the button is not one of the default controls (they are formattingControls: [ 'bold', 'italic', 'link', 'strikethrough' ]) and no blocks currently use the button.
Adding the button here just opens the possibility for a block to have an underline button if the block wants to by changing the formattingControls prop. If we don't add the button here even if the block wants to show the underline the button will not render.
There was a problem hiding this comment.
Ah, so it's mainly for third-party blocks that might want to extend RichText and show that button.
That makes sense, but then a comment mentioning why it's there/not rendered by default (basically what you said here) would be really helpful in explaining why it's there 😄
There was a problem hiding this comment.
Given that in the past the button was explicitly removed and just the keyboard shortcode was allowed, other blocks should follow the same approach and we don't expect blocks to have the underline button so I end up removing the button as suggested by @iseulde and @tofumatt.
If a block really wants the button it is still possible using a custom format.
| export const underline = { | ||
| name, | ||
| title: __( 'Underline' ), | ||
| tagName: 'u', |
There was a problem hiding this comment.
Let's use a span with inline styles, just as we did in the classic editor.
This element used to be called the "Underline" element in older versions of HTML, and is still sometimes misused in this way. To underline text, you should instead apply a style that includes the CSS text-decoration property set to underline.
There was a problem hiding this comment.
I updated the code to use an inline style, thank you for pointing this. Now the markup is exactly the same as the one used by the classic editor.
jorgefilipecosta
force-pushed
the
fix/undeline-text-when-cmd-u-is-pressed
branch
from
6428563
to
1532375
Compare
jorgefilipecosta
force-pushed
the
fix/undeline-text-when-cmd-u-is-pressed
branch
from
1532375
to
d5ea4f3
Compare
| }; | ||
|
|
||
| return ( | ||
| <Fragment> |
There was a problem hiding this comment.
Is the Fragment wrapper necessary?
|
This no longer works since #12249. |
Description
Fixes: #13034
This PR adds a core/underline format option. This format option makes sure that when cmd + u is pressed the selected text gets the underline format.
This shortcut was documented but was not implemented.
How has this been tested?
I tested in chrome and firefox that when I press cmd + u inside a paragraph (or other block containing a RichText component e.g: image caption) the selected text gets underline format.