WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 2 years ago

#36569 closed defect (bug) (fixed)

Remove Link button in Visual Editor needs to be disabled when cursor isn't over link

Reported by: ahortin Owned by: melchoyce
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.2
Component: TinyMCE Keywords: has-patch
Focuses: ui, javascript Cc:

Description

In previous WP versions, when your cursor wasn't sitting on top of a link in the Visual Editor, the Remove link icon in the toolbar was disabled, indicating that it was unavailable. When the cursor was placed over a link, it would then become available.

It's now showing as enabled all the time, even when your cursor is not on a link. This indicates, wrongly, that there is some action that it can perform.

As can be seen in the following pic, the Remove link icon is the same colour as the Insert/edit link icon.

https://cloudup.com/cTXFPpj4eLF

Looking at the Redo button, it has the class mce-disabled added to its containing div, changing the icon to a light grey, indicating that it's not available. The Remove link icon should operate in the same way (which is how it used to work).

Attachments (1)

36569.patch (1.4 KB) - added by azaozz 2 years ago.

Download all attachments as: .zip

Change History (12)

#1 @swissspidy
4 years ago

  • Component changed from Editor to TinyMCE

I wonder when this changed. Who wants to find out? :)

Looks like we can't just use disabledStateSelector: ':not(a):not([href])' because TinyMCE doesn't support these selectors.

#2 @ahortin
4 years ago

@swissspidy Looks like v4.2 from what I can tell. I just tested 4.1.10 and it was working fine and then after upgrading to 4.2 the icon would no longer display the disabled state.

#3 @azaozz
4 years ago

  • Keywords ux-feedback added

This is the TinyMCE default behaviour. The problem is that in some cases it is hard to detect when:

  • Part of a link and some text are selected.
  • A link is in the middle of the selection.
  • More than one link is selected.
  • Parts of two links are selected.

In these cases running the contentEditable unlink command will properly remove the link(s) however it may fail to enable the unlink button. Also note that TinyMCE doesn't use the unlink button any more. Links can also be removed by editing them and deleting the URL.

I'm on the fence if we should be changing TinyMCE's default. We can easily disable the button on loading and only enable it when the caret or the selection is in an <a> node. That will leave it disabled in the above four cases.

Since 4.3 links can also be removed from the inline toolbar, by clicking on them and clicking the X button. So maybe we should remove the toolbar Unlink button too.

Last edited 4 years ago by azaozz (previous) (diff)

#4 @ocean90
4 years ago

  • Version changed from trunk to 4.2

#5 follow-up: @afercia
4 years ago

  • Focuses ui javascript added; accessibility removed

I'd rather consider to remove the "unlink" button because... it's one button less ;) Seriously, now it just duplicates the "X" remove button in the inline toolbar, right? And maybe the "X" button could use the "unlink" icon.

https://cldup.com/5j6taod9GS.png

Removing the accessibility focus since not strictly related to a11y.

#6 in reply to: ↑ 5 @azaozz
4 years ago

Replying to afercia:

I'd rather consider to remove the "unlink" button because... it's one button less ;)

Yep, my thinking too :)

...maybe the "X" button could use the "unlink" icon.

I think it uses the X to stay consistent with the wpViews (oEmbed previews, galleries, etc.), but having the "unlink" icon there would be better. That will also fix #37278 about X not being intuitive enough.

Last edited 4 years ago by azaozz (previous) (diff)

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


2 years ago

#8 @melchoyce
2 years ago

Since the "unlink" icon appears contextually, I'd be fine with removing it from the TinyMCE toolbar.

#9 @melchoyce
2 years ago

  • Keywords needs-patch added; ux-feedback removed

@azaozz
2 years ago

#10 @azaozz
2 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 4.9

In 36569.patch: removing unlink from the editor toolbar is as easy as that :)

Last edited 2 years ago by azaozz (previous) (diff)

#11 @melchoyce
2 years ago

  • Owner set to melchoyce
  • Resolution set to fixed
  • Status changed from new to closed

In 41831:

TinyMCE: Remove "Unlink" icon from toolbar

Because "unlink" now appears contextually when editing a link, let's remove it from the toolbar.

Props azaozz, ahortin, swissspidy, afercia.
Fixes #36569.

Note: See TracTickets for help on using tickets.