WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#33301 closed task (blessed) (fixed)

Create and edit links inline

Reported by: iseulde Owned by: azaozz
Milestone: 4.5 Priority: normal
Severity: normal Version:
Component: Editor Keywords: dev-feedback has-patch
Focuses: ui, accessibility, javascript, administration Cc:

Description

The current idea is to merge the URL input and the search field together and detect when the user enters a URL or tries to search for one. This is similar to what Google Docs does. Search results should be shown as an autocomplete dropdown. Ideally this is extendible by plugins, e.g. they could add search engine results or categories of the blog. Any other fields, such as the target, title attribute or anything else a plugin wants to add could be under a "more options" or "advanced" toggle.

Attachments (16)

33301.patch (4.1 KB) - added by iseulde 3 years ago.
33301.2.patch (4.9 KB) - added by iseulde 3 years ago.
33301.3.patch (8.5 KB) - added by iseulde 3 years ago.
33301.4.patch (10.2 KB) - added by iseulde 3 years ago.
33301.5.patch (10.5 KB) - added by iseulde 3 years ago.
33301.6.patch (10.7 KB) - added by iseulde 3 years ago.
33301.7.patch (11.5 KB) - added by iseulde 3 years ago.
33301.8.patch (11.4 KB) - added by iseulde 3 years ago.
33301.9.patch (12.2 KB) - added by iseulde 3 years ago.
33301.10.patch (12.1 KB) - added by iseulde 3 years ago.
33301.11.patch (12.1 KB) - added by iseulde 3 years ago.
33301.12.patch (12.1 KB) - added by iseulde 3 years ago.
33301.13.patch (21.5 KB) - added by azaozz 3 years ago.
33301.14.a11y.patch (8.9 KB) - added by afercia 3 years ago.
33301.15.patch (1.6 KB) - added by afercia 3 years ago.
33301.voiceover.diff (1.4 KB) - added by afercia 3 years ago.
a11y help for VoiceOver

Download all attachments as: .zip

Change History (111)

@iseulde
3 years ago

#1 @iseulde
3 years ago

Ignore patches. Just saving work.

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

#2 @azaozz
3 years ago

  • Milestone changed from Future Release to 4.5

33301.12.patch looks good, thinking it's ready for a "first-run". Having it in core will make testing easier. Can make it work in IE after the UI/UX review.

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

#3 @afercia
3 years ago

  • Focuses accessibility added
  • Keywords has-patch has-screenshots added

Some patch refreshes late, a couple of hours ago tested 33301.9.patch for a11y and works nicely, see screenshot :)

https://cldup.com/ZsNyUtqWD0.png

#4 @iseulde
3 years ago

In 36384:

TinyMCE: add inline link dialog

First run.
Links the advanced button to the "old" dialog for now.

See #33301.

#5 follow-up: @afercia
3 years ago

Sorry, just realised there's one thing missing for accessibility: the search field should have an associated label to describe its purpose. Currently, screen readers announce the search field just as:

combo box  collapsed  has auto complete  editable
blank

The best option would be a real <label> element, not sure it would be so easy to implement. Alternatively, an aria-label attribute would be enough, I think. Something like:

'role': 'combobox',
'aria-label': 'Search for existing content or enter a link',

Maybe with a better wording :) and translatable, of course.

Last edited 3 years ago by afercia (previous) (diff)

#6 follow-up: @iseulde
3 years ago

Is aria-label enough or is a label always better?

#7 in reply to: ↑ 6 ; follow-up: @afercia
3 years ago

Replying to iseulde:

Is aria-label enough or is a label always better?

Both are allowed, in this case I'd probably go with aria-label, see: Labeling Controls https://www.w3.org/WAI/tutorials/forms/labels/

#8 in reply to: ↑ 7 @azaozz
3 years ago

Replying to afercia:

If both are allowed and work well, I'd go for aria-label as we don't need a <label> element there. The context is obvious. Or we can go for <label> with offscreen-hidden text if needed.

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

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 years ago

#10 in reply to: ↑ 5 ; follow-up: @DrewAPicture
3 years ago

Replying to afercia:

Sorry, just realised there's one thing missing for accessibility: the search field should have an associated label to describe its purpose.

I think visual users could benefit from something like an input placeholder, this would be in addition to an aria-label for screen reader users.

I encountered the new inline dialog this morning and my first impression was, "yuck, now it takes another click to get to the full wplink dialog". But then I read this ticket and realized that what I assumed was the URL input only now supports autocomplete, which is really cool — but not obvious at all. There was nothing to indicate that it was anything other than a URL input.

Let's make it more obvious.

#11 in reply to: ↑ 10 @michaelarestad
3 years ago

Replying to DrewAPicture:

I think visual users could benefit from something like an input placeholder, this would be in addition to an aria-label for screen reader users.

Can we make it a placeholder example based on a recent post or page?

#12 @DrewAPicture
3 years ago

Some other feedback in addition to comment:10:

  • I wonder if we might be better served to use a check mark icon instead of what I guess is supposed to be a apply or enter icon? I don't think I've ever seen that icon elsewhere in the admin
  • Removing links now seems stilted. Previously I could click a link then click the X and it would remove the link. Now it seems like the only way to remove a link is to open the full dialog and close it again. Or I guess select the contents of the input and delete it. Not really obvious.
Last edited 3 years ago by DrewAPicture (previous) (diff)

#13 @azaozz
3 years ago

Can we make it a placeholder example based on a recent post or page?

We can pass the last published post or page title to it. However not sure we will be able to fit all the text. The placeholder should probably be: Paste URL or type to search existing content. How do you see using the last post title in there? Or am I misunderstanding completely? :)

Removing links now seems stilted. Previously I could click a link then click the X and it would remove the link.

This still works in exactly the same way. Clicking on a link inside the editor opens the "Follow link" mode, clicking on the pencil icon there switches it to editing mode. Are you seeing something different?

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

#14 @afercia
3 years ago

Am I the only one having troubles in Firefox and IE (also Edge) when using only the keyboard? In Chrome works fine, in other browsers it's basically impossible to handle it correctly. Sorry I really don't know anything about the way TinyMCE attaches events but I'd suggest to have a look at what the old suggest.js does in the processKey() function to handle events when pressing Enter on the suggested terms. Bit tricky since the focused element is always the input field so when the results dropdown is visible, events on the input field should be probably prevented.

This ticket was mentioned in Slack in #core by mike. View the logs.


3 years ago

#16 @afercia
3 years ago

Just noticed, the z-index nightmare :)

https://cldup.com/gsaQ_1Zphc.png

#18 @iseulde
3 years ago

Thanks @ryan!

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 years ago

#20 @djp424
3 years ago

  • Focuses ui administration added
  • Keywords needs-patch added; has-patch has-screenshots removed
  • Type changed from enhancement to defect (bug)
  • Version set to 4.4.1

Bug: If you add a new link without having any text selected or your cursor over any text, after you enter your url, the link will disappear after you type text in.

Steps to reproduce:

  1. add new post
  2. go to new line
  3. click on create link
  4. type in link
  5. click enter
  6. try to type something (you will notice that the link disappears) {In addition, JS will display url dialog if you put cursor to the left of the text you type in after step 5, however nothing is inserted in the editor itself. If you click on the "Text" tab and back on "Visual", it will go away.}

Suggested fix: that it creates a link with the text being the url you put in.

Example: If you go to new line, add new link, and submit google.com, the text should be google.com and it should link to http://google.com.

Let me know what your thoughts are.

Video: https://cloudup.com/cJU1a6Jhn7H

Last edited 3 years ago by djp424 (previous) (diff)

This ticket was mentioned in Slack in #core-editor by justice. View the logs.


3 years ago

#22 @ericlewis
3 years ago

  • Type changed from defect (bug) to enhancement
  • Version 4.4.1 deleted

#23 follow-up: @azaozz
3 years ago

In 36483:

TinyMCE inline link:

  • Fix not displaying anything when the URL is only a fragment. Show the whole URL.
  • Fix editing a link when it is the very first word in the editor.
  • Fix editing a link then some of the surrounding text or space is selected. Change the selection to only the link node.
  • Add placeholder when adding new link.

See #33301.

#24 in reply to: ↑ 23 @afercia
3 years ago

Replying to azaozz:

  • Add placeholder when adding new link.

Happy to see the latest changes greatly improve keyboard interaction and solve the main issues noticed in Firefox and IE and mentioned in comment 14 :) Noticed some more things to check:

Minor visual thing:

  • place the cursor on an already existing link
  • the link toolbar appears
  • press Tab to move away from the Editor
  • the link toolbar doesn't disappear

Initial focus:

  • place the cursor on an already existing link
  • edit the link either by clicking the pencil icon in the link toolbar or using a keyboard shortcut (e.g. Ctrl/Cmd+k)
  • the initial focus is on the input field
  • press Tab to move focus to the "Apply" or "Advanced" buttons
  • press Escape to close the toolbar
  • edit again the link
  • the initial focus is on the previously focused button

Expected: initial focus should always be set on the input field and not on the "toolbar".

Search results and jQuery autocomplete live regions extension quirks:

  • When users manually type or paste a URL the search shouldn't be triggered. Maybe detecting when input value starts with "http"?
  • When editing a link (i.e. the input field has some value), each time the input field get focused a live region message "No search results" gets announced. This happens also when moving to the "Apply" or "Advanced" buttons and then moving back to the input field
  • Same, when pasting a URL a message "No search results" gets announced

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 years ago

This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.


3 years ago

#27 @afercia
3 years ago

Tested a bit using Internet Explorer 11 (Windows 10) and noticed there's something wrong when the placeholder gets injected, to reproduce:

  • select some text or just place the cursor in the middle of a word
  • press either Shift+Alt+a or Ctrl+k

as soon as you do this, the cursor is moved out of the selection so the toolbar doesn't appear. Seems the placeholder gets injected though (the text becomes underlined).

Edit: same when clicking on the "Insert/edit link" button.

Last edited 3 years ago by afercia (previous) (diff)

#28 @azaozz
3 years ago

In 36602:

TinyMCE, inline link dialog:

  • Fix passing values to the (old) modal on open when non-linked text is selected and Advanced is clicked before pasting an URL.
  • When the user has selected text partially including a link and opens the editing dialog, auto-select the link only. Helps when a linked word is selected by double-clicking.
  • Remove all placeholders on saving.
  • Do not add undo level on inserting link placeholder.
  • Remove the placeholder when canceling from the modal.

See #33301.

#29 @ocean90
3 years ago

In 36605:

JSHint for [36602].

See #33301.

@azaozz
3 years ago

#30 @azaozz
3 years ago

In 33301.13.patch:

  • Remove the bottom part (river/infinite scroll) from the old wplink dialog and replace it with autocomplete on the URL field. This makes it consistent with the inline dialog and leaves more space for plugins to extend it.
  • Some css and js cleanup and minor fixes.

This ticket was mentioned in Slack in #core-editor by azaozz. View the logs.


3 years ago

#32 @ocean90
3 years ago

  • Owner set to azaozz
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core by jorbin. View the logs.


3 years ago

#34 @jorbin
3 years ago

  • Type changed from enhancement to task (blessed)

Almost there. Making a task to complete the few remaining bits during beta.

#35 @azaozz
3 years ago

In 36677:

TinyMCE, inline link dialog:

  • Remove the bottom half of the (old) modal and add autocomplete on the URL field.
  • Disable the inline edit dialog in old IE (7, 8 and 9). Use only the modal there.
  • Fix in IE10 and 11.
  • Fix (most?) remaining edge cases.
  • Fix focusing the inline dialog, the modal and the editor.

See #33301.

#36 follow-up: @afercia
3 years ago

Great improvements with the last changes, especially for IE :) Noticed a new issue though, when the input field is focused it's no more possible to tab to the "Apply" and "Advanced" buttons.

#37 @azaozz
3 years ago

In 36703:

TinyMCE, inline link dialog:

  • Reset the inline dialog when canceling the advanced modal. If there is a link it should be on the first stage: follow/preview link.
  • Fix tabbing in the inline edit dialog.

See #33301.

#38 in reply to: ↑ 36 @azaozz
3 years ago

Replying to afercia:

Tabbing should be fixed now. Was caused by using <input type="hidden"> which seems to break the JS handling of the tab key.

#40 @azaozz
3 years ago

In 36716:

TinyMCE, inline link:

  • Make sure the inline dialog is not showing under the advanced modal.
  • Fix checking if the link node contains text.
  • Fix undo levels so all actions can be undone and redone.

See #33301.

#41 in reply to: ↑ 39 @azaozz
3 years ago

@rianrietveld thanks for the tests!

There were some problems with focusing the URL field on opening the inline dialog, should be fixed after [36716]. Also focusing the URL field in the advanced modal should always work now. That should fix many of the reported problems.

Still to-do: fix using the Enter key in Firefox in the URL field. Not sure if the spacebar can be used there to trigger "apply". It is a text field, we shouldn't disable the spacebar even though spaces are not legal in URLs.

Also not sure what to do about some apps not announcing the opening of the dialog. Do we need to add something that would trigger it?

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

#42 @afercia
3 years ago

Sorry for the confusion about the Spacebar. The original issue reported was:

Win 10 using FF and Keyboard. Keyboard: I also forgot to mention that using the spacebar instead of Enter when creating a link using the keyboard will make it so a line break does not occur after the text link.

I assumed it was about the "Apply" button, which seems to work for me also with Spacebar. Maybe at some point it was also inserting a space after the link?

By the way, one of the main issues is initial focus. Using the keyboard, the first time you open the toolbar, focus correctly goes to the input field. This is extremely important because the input field gets announced as:

Paste URL or type to search combo box collapsed has auto complete editable

giving this way a complete information to users. (ignore "has auto complete" which is announced for any input field and has to do with the browser feature to remember what you entered in input fields).

After the first time though, focus goes to the last element that had focus in the toolbar. So if users tabbed to "Apply" or "Advanced", when they open again the toolbar they will hear just:

Toolbar Apply button

which is pretty confusing. They will have to figure out they have to tab backwards to find the input field.

As far as I see this is a TinyMCE toolbars "feature", for example it happens also in the main buttons toolbar. To reproduce:

  • press Alt+F10 and then tab, say, to the "Blockquote" button
  • press Escape to move focus back to the editing area
  • press Alt+F10 again
  • TinyMCE "remembers" the last focused element in the toolbar and will move focus to the "Blockquote" button

While this feature may (or may not?) be useful in the buttons toolbar, it's really confusing in the link inline toolbar. I'd recommend to try to disable it or try to always force initial focus on the input field.

#43 @afercia
3 years ago

Here's a couple videos, both recorded today on updated trunk local installations. First, the good news: with Firefox + NVDA (Windows 10) works pretty well https://cldup.com/S-YNfBdB3y.mp4

There are a few things I'd recommend to consider:

  • when using the arrow keys to highlight the search results, what you've typed in the input field, in this case "templa", shouldn't get removed
  • the "highlighted" search results should have a much stronger color contrast, maybe inverting the colors?
  • at the end, when a link is successfully inserted, it's pretty clear the lack of an audible confirmation message

Safari + VoiceOver, courtesy of @rianrietveld https://cldup.com/43Q5kdQA5L.mp4

  • looks like the initial focus has a weird behavior, not sure what's really happening, sorry I'm afraid I can't be of much help
  • when using the arrow keys to highlight the search results, it just reads out the content of the input field ("sam") and then nothing (because "sam" gets removed); my only guess here it's that VoiceOver doesn't switch to "forms mode" mode for some reason [edit: looks like I'm not a VoiceOver expert at all :)]

Additional text with IE 11, not related to screen readers:

  • looks like the link insertion works only when placing the cursor in the middle of a word in the editor; instead, when selecting one or multiple words, the link doesn't get inserted
Last edited 3 years ago by afercia (previous) (diff)

#44 @azaozz
3 years ago

In 36743:

TinyMCE, inline link:

  • Fix applying the changes when pressing the Enter key in Firefox. No longer inserts new paragraph in the editor.
  • Fix empty check when getting text from the dialog.
  • Always focus the URL field when opening the dialog.
  • Add back the keydown events in the modal.

See #33301.

#45 @azaozz
3 years ago

In 36747:

TinyMCE, inline link:

  • Fix in IE (again). Remove setting/getting placeholders, pass the link node instead.
  • In the inline dialog: when the selected text looks like URL or email, pre-fill the URL field with it (same as in the modal).
  • Fix setting the name of the main button in the modal: Add Link or Update.
  • In the modal when clicking Update remove the link if the URL field is empty. That matches the inline dialog behaviour. Otherwise the modal remains open, nothing happens when clicking the Update button there.

See #33301.

#46 @johnbillion
3 years ago

In 36751:

Editor: Remove an unused JavaScript variable so the JS lint tests pass.

See #33301

#47 @jadpm
3 years ago

Since 36677 the wplink.js script is directly dependent from the global tinymce. Before that, that global was indeed declared but we checked against its existence when used in the open method - the only place where it was used indeed.

That has caused a regression in some implementations, and I am thinking about Quicktags. By its own inline documentation,

This (quicktags.js) is the HTML editor in WordPress. It can be attached to any textarea and will.

From now on, adding Quicktags with a link button to a custom HTML textarea outside the editor pages lacks the global tinymce unless the target script is added also as a dependency. Ys, Quicktags do not have wplink.js as a dependency either, since link buttons are not mandatory on a quicktags toolbar, but one would expect that when loading ths script it can be used as-is. That is not the case anyore.

So, as a summary, we might want to consider that there are some implementations where wplink is used on admin pages where tinyMCE is not loaded or even needed.

In my opinion, we should either add the tinymce script as an actual dependency to wplink.js so it loads automatically, wrap all its related functionality in checks like it did before or move all the related code to another proper script.

#48 @azaozz
3 years ago

@jadpm right, there is no need for wplink.js to be dependent on tinymce.js, and for the TinyMCE plugin to be dependent on wplink.js. Patch coming up.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 years ago

#50 @azaozz
3 years ago

In 36777:

TinyMCE, inline link:

  • Fix running wpLink without tinymce.js and the TinyMCE plugin without wplink.js.
  • Do not show the Advanced button in the inline link dialog when wpLink is not loaded.

See #33301.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 years ago

#52 follow-up: @MarcoZ
3 years ago

I just gave the trunk installation a quick test with both Safari and Chrome on OS X with VoiceOver (not ChromeVox), and find that Chrome does read the search results. But due to a bug in its text interface, it doesn't read anything you type in any text field, nor does it re-read the text when arrowing over it. But the search result arrowing up and down does work.

I could not confirm that focus would not land in the combobox for me in Safari after CMD+K.

Without having looked at the code, here are a few ideas:

  • If you use aria-activedescendant to manage focus for the individual option items in the auto-complete, but have not added tabindex="-1" to make them focusable but not included in the tab order, please try adding those and see if it makes a difference.
  • If you use aria-owns, this may interfere, because it is inconsistently implemented. Try removing that and use aria-controls instead, or exclusively, to point from the combobox itself to the container holding the option items.
  • Or are you only using a live region to speak the result arrowed to? If so, you might want to consider using a real widget markup instead, since live region support has been known to be somewhat inconsistent still.

Hope these help!

#53 @azaozz
3 years ago

In 36806:

TinyMCE, inline link:

  • Add uiAutocompleteL10n with translatable strings for use in UI Autocomplete live region.
  • Use the above strings in both the editor plugin and wplink.js.

See #33301.

#54 in reply to: ↑ 52 ; follow-up: @afercia
3 years ago

Replying to MarcoZ: Thanks very much Mr. Zehe :) The current implementation uses a live region but only to announce the total number of results. The combobox has an aria-activedescendant attribute and it seems it gets correctly updated. The combobox aria-owns points to an unordered list with role="listbox". The option items use tabindex="-1". Example of one option item markup:

<li role="option" id="mce-wp-autocomplete-1011" class="ui-menu-item" tabindex="-1"><span>This is one search result</span> <span class="alignright">2012/03/15</span></li>

Tested with Firefox + NVDA and IE 11 + JAWS and it works pretty well. Fails miserably with VoiceOver. We'll try to change aria-owns or maybe could it be role="combobox" on the input field? I've noticed the Twitter search omits it. Thanks again!

This ticket was mentioned in Slack in #core-editor by azaozz. View the logs.


3 years ago

#56 in reply to: ↑ 54 @MarcoZ
3 years ago

Replying to afercia:

Tested with Firefox + NVDA and IE 11 + JAWS and it works pretty well. Fails miserably with VoiceOver. We'll try to change aria-owns or maybe could it be role="combobox" on the input field? I've noticed the Twitter search omits it.

Could be either, I suggest removing these in turn and see if either one fixes the problem. I've noticed whacky things going on with comboboxes even in Apple's own COCOA widgets in other applications where VoiceOver doesn't read them well, so going the Twitter approach might be the right thing to do.

Alternatively, on Twitter, Léonie Watson mentioned this design pattern by Gez Lemon, which works with VoiceOver.

This ticket was mentioned in Slack in #polyglots by sergey. View the logs.


3 years ago

#58 @azaozz
3 years ago

In 36849:

TinyMCE, inline link: add styling for the dialog and UI Autocomplete to Press This.

See #33301.

#59 @afercia
3 years ago

Looks like jQuery UI autocomplete is adding two ARIA live regions, see screenshot below. As far as I see, it actually uses the second one (thanks jQuery) so maybe it's not critical. Not sure why this happens, sorry I can't help so much here :)

https://cldup.com/sTkCnu0A_a.png

#60 follow-up: @afercia
3 years ago

  • Keywords dev-feedback added

The original jQuery UI autocomplete, see https://jqueryui.com/autocomplete, populates the search field with the highlighted option value when using a keyboard. To reproduce, use your keyboard and type "a" in the search field on https://jqueryui.com/autocomplete/ and then:

  • use the down/up arrow key on your keyboard to highlight the search results
  • notice the field gets populated

In the current WP implementation this doesn't happen because jQuery UI autocomplete expects an item.value which is undefined. Thus, the search field gets emptied.

By the way, neither of these two behaviors is ideal for accessibility. The input field value should never be changed while using the arrow keys. When users type, say, "des" and then use the arrow keys but then they want to refine the search completing what they were typing (say "des" and then "ign") , they will find the search field empty and will be forced to start typing the search term from scratch.

This is especially confusing when using a screen reader, I'd strongly recommend to try to override what happens when the menu items are focused. See menufocus in jQuery UI autocomplete. Any help would be greatly appreciated :)

Edit: Found http://api.jqueryui.com/autocomplete/#event-focus

Canceling this event prevents the value from being updated, but does not prevent the menu item from being focused.

Last edited 3 years ago by afercia (previous) (diff)

#61 in reply to: ↑ 60 @azaozz
3 years ago

Replying to afercia:

The original jQuery UI autocomplete, see https://jqueryui.com/autocomplete, populates the search field with the highlighted option value when using a keyboard. In the current WP implementation this doesn't happen.

Right, and inserts the originally typed string back when returning to the input field with the arrow keys.

By the way, neither of these two behaviors is ideal for accessibility. Found http://api.jqueryui.com/autocomplete/#event-focus

Canceling this event prevents the value from being updated, but does not prevent the menu item from being focused.

If not changing the input field value while using the arrow keys is best for accessibility, lets do that.

On the other hand the default behaviour seems better when hovering over the menu as it gives feedback what URL is going to be inserted. Shall I try to add that when not using the arrow keys?

Edit: hmm, or better not. It's somewhat distracting to change the input field content on hovering over the menu.

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

#62 @azaozz
3 years ago

In 36974:

TinyMCE, inline link: when searching, do not empty the URL input field when using the arrow keys to highlight items.

Props afercia.
See #33301.

#63 @afercia
3 years ago

Thanks @azaozz :) While testing keyboard accessibility, noticed something about "Undo" and "Redo". They perfectly work when clicking on the "Undo" and "Redo" buttons. By the way, when using the associated shortcuts "Cmd/Ctrl+Z" and "Cmd/Ctrl+Y" they don't always work. To reproduce:

  • insert a link and then edit it again, change the link and apply
  • use "Cmd/Ctrl+Z" to undo
  • at some point the keyboard focus will be inside the input field and shortcuts don't work there

Same when re-doing. Sorry I can't help so much, know very little about TinyMCE. I've tried to call editor.undoManager.undo() and editor.undoManager.redo() on the keydown event on the input field and it worked. There are probably better ways to do it though :)

#64 @azaozz
3 years ago

In 36977:

TinyMCE, inline link: ensure the inline dialog is in preview mode after updating a link from the (advanced) modal.

Props afercia.
See #33301.

#65 @afercia
3 years ago

Would like to propose a few improvements for accessibility and interaction with screen readers. 33301.14.a11y.patch is a first pass. Removing a couple of tabindex attributes that may confuse screen readers. Also, adds audible confirmation messages using wp.a11y.speak(). A few more changes, all commented in the patch. Couple things could certainly be better coded, please do feel free to improve them. Any thoughts more than welcome :)

#66 @azaozz
3 years ago

In 36982:

TinyMCE, inline link: when doing undo/redo with keyboard shortcut, do not focus the inline dialog. Cannot do consecutive undo/redo if the focus is moved away from the editor.

See #33301.

#67 @azaozz
3 years ago

In 36984:

TinyMCE, inline link:

  • Add audible confirmation when a link has been selected or inserted in the editor for both the inline dialog and the modal.
  • Do not auto-search when the URL field is empty or already contains an URL.
  • Remove a few redundant tabindex.

Props afercia, azaozz.
See #33301.

#68 @azaozz
3 years ago

In 36985:

TinyMCE, inline link:

  • Remove unused user setting for wpLink.
  • Remove redundant text and variable from wp_link_dialog().

Props afercia, azaozz.
See #33301.

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


3 years ago

#70 follow-up: @melchoyce
3 years ago

Continuing to look good.

Some thoughts:

  1. Did we want to go with a checkmark icon instead of the enter icon, per this convo? https://wordpress.slack.com/archives/core-editor/p1453830667000118
  2. If not, can we move the enter icon 1px to the left?
  3. Can we tighten up the spacing between the icons in both editing and preview mode? On edit, I think we can remove the entire margin-left on the cog (and leave the margin-right on the button). There's also 4px of margin between the edit/remove icons in preview mode that I think we can eliminate entirely, along with the left margin on edit and right margin on remove to bring everything in just a little.
  4. Because of the thicker bottom border on the button, the button is taller than the input. Can we adjust it so they're the same height?

#71 follow-up: @azaozz
3 years ago

Did we want to go with a checkmark icon instead of the enter icon?

Seems it was "undecided" at the end because:

In Korea and Japan, the O mark is used instead of the check mark. In Japan the check mark is commonly used instead of an X for wrong. (https://en.wikipedia.org/wiki/Check_mark)

Another question: are we keeping "Advanced" as tooltip for the cogwheel button that opens the (old) modal? Maybe something like "Link options" or "Link attributes" or "More settings" would be better?.

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

#72 in reply to: ↑ 71 ; follow-up: @melchoyce
3 years ago

Replying to azaozz:

Seems it was "undecided" at the end because:

In Korea and Japan, the O mark is used instead of the check mark. In Japan the check mark is commonly used instead of an X for wrong. (https://en.wikipedia.org/wiki/Check_mark)

Can we have someone from Japan and Korea confirm this? That page has zero references or citations.

Another question: are we keeping "Advanced" as tooltip for the button that opens the (old) modal? Maybe something like "Link options" or "Link attributes" or "More settings" would be better?.

I'd go with "Link Options."

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


3 years ago

#74 follow-up: @azaozz
3 years ago

In 37001:

TinyMCE, inline link:

  • Remove bottom box-shadow from the Apply button so it matches the rest.
  • Change the tooltip for the cogwheel button to Link options.

See #33301.

#75 in reply to: ↑ 70 ; follow-up: @azaozz
3 years ago

Replying to melchoyce:

  1. If not, can we move the enter icon 1px to the left?
  2. Can we tighten up the spacing between the icons in both editing and preview mode?

Didn't do these in the above patch as they come from the "main" toolbar and are the same in the inline Edit Image toolbar. I mean, if they need tightening here, maybe better to tighten them everywhere.

#76 @azaozz
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

This should be complete. New tickets for any bugs or edge cases please :)

#77 in reply to: ↑ 74 @melchoyce
3 years ago

Replying to azaozz:

In 37001:

TinyMCE, inline link:

  • Remove bottom box-shadow from the Apply button so it matches the rest.
  • Change the tooltip for the cogwheel button to Link options.

See #33301.

Sorry, should have been more clear — I don't think the box-shadow should be removed from the Apply button, but instead the button itself should become shorter to compensate for the size difference.

#78 in reply to: ↑ 75 @melchoyce
3 years ago

Replying to azaozz:

Replying to melchoyce:

  1. If not, can we move the enter icon 1px to the left?
  2. Can we tighten up the spacing between the icons in both editing and preview mode?

Didn't do these in the above patch as they come from the "main" toolbar and are the same in the inline Edit Image toolbar. I mean, if they need tightening here, maybe better to tighten them everywhere.

I disagree. The toolbar, given its dense amount of tools, should be spacious. The inline linking toolbar only has two icons and does not need to be that spacious, and would benefit from a decrease in space around the icons. Visually, having the icons so separate within the inline linking toolbar looks unbalanced and off. I think we should make an exception for it based on the context.

#79 @azaozz
3 years ago

In 37004:

TinyMCE, inline link:

  • Add back the bottom box-shadow on the Apply button.
  • Shrink .mce-btn.mce-primary to compensate for the bottom box-shadow.
  • Tighten up/reduce the margins between the buttons.

See #33301.

#80 in reply to: ↑ 72 @afercia
3 years ago

Replying to melchoyce:

In Korea and Japan, the O mark is used instead of the check mark. In Japan the check mark is commonly used instead of an X for wrong. (https://en.wikipedia.org/wiki/Check_mark)

Can we have someone from Japan and Korea confirm this? That page has zero references or citations.

https://www.w3.org/standards/webdesign/i18n

The check mark means correct or OK in many countries. In some countries, however, such as Japan, it can be used to mean that something is incorrect.

I think the same applies to some Northern Europe countries, about Norway we could ask @clorith

#81 @Clorith
3 years ago

I've never seen a check mark represent a negative state my self (In tests they represent a correct answer, when I'm doing tasks they mean "completed"), but that just covers Norwegian usage (My Finnish friend said they can go both ways in Finland depending on context).

Unfortunately the return symbol might be too old-school for a lot of users to recognize the meaning of, this button might benefit from actually being labeled over icon-only?

If it has to be an icon, the check mark is probably the most universally known "OK, I'm done", although it feels ab it misplaced in this setting in my opinion.

@afercia
3 years ago

#82 @afercia
3 years ago

  • Keywords has-patch added; needs-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Discussed a bit on Slack to reopen and see which accessibility improvements can be done at this point of the release, after the "advanced modal dialog" was reverted. Worth considering all the search results part (the area highlighted in the screenshot below) is not accessible: nothing gets announced by screen readers.

https://cldup.com/KWAAiH7Nf3.png

Unfortunately too many changes would be necessary to fix this and at this point of the release that's not an option. It is not a regression though.

I'd recommend just two minor, slight, improvements adding a couple of aria-describedby attributes to make the URL and Search fields descriptions being read out when the fields are focused. See 33301.15.patch

This ticket was mentioned in Slack in #core-editor by iseulde. View the logs.


3 years ago

#84 @azaozz
3 years ago

  • Keywords commit added

33301.15.patch looks good. I know it is very late, but it only adds couple of ARIA attributes to fix some accessibility problems in the part that was reverted.

#85 @afercia
3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 37160:

Accessibility: improvements for the Editor wpLink modal form fields.

Adds aria-describedby attributes to the modal form fields after it
was partly restored in [37154].

Fixes #33301.

#86 @afercia
3 years ago

About the issue with Safari, I've recently had the opportunity to test using Safari+VoiceOver and noticed the Twitter search suggestion (which perfectly works) sets an aria-selected="true" on the highlighted option item. Quickly tried that and it seems it makes Safari understand what's going on, finally the autocomplete items get read out. Need to build a clean patch and test a lot but we're probably on the right track now :)

Can confirm that in Safari after Cmd+K the focus goes forth and back from the Editor to the combobox 2-3 times, sometimes it lands on the search field, sometimes not. Couldn't reproduce consistently. Please consider I'm a newbie with VoiceOver, but I've noticed this focus "jump" doesn't happen when I diligently follow VoiceOver instructions telling me: "To enter the web area, press Control-Option-Shift-Down Arrow". After this, everything works nicely. Would greatly appreciate some feedback from VoiceOver experienced users, specifically if pressing "Control-Option-Shift-Down Arrow" enters a sort of "forms mode" and affects how JavaScript keyboard events behave. @MarcoZ any thoughts? When you have a chance! Thanks very much :)

This ticket was mentioned in Slack in #core by afercia. View the logs.


3 years ago

@afercia
3 years ago

a11y help for VoiceOver

#88 @afercia
3 years ago

In my testing, 33301.voiceover.diff makes Safari + VoiceOver read out the autocomplete items. Would greatly appreciate some testing and a JS review, couldn't find a cleaner way to remove the aria-selected attribute.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 years ago

#90 @azaozz
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for a quick consideration of the accessibility fix.

33301.voiceover.diff looks good here. I can't think of anything that might break by adding/removing another attribute on the selected UI Autocomplete menu item, but at the other hand... 3 days before release? :)

+1 from me anyway.

This ticket was mentioned in Slack in #core by mike. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by mike. View the logs.


3 years ago

#93 @jeremyfelt
3 years ago

  • Keywords commit removed
  • Resolution set to fixed
  • Status changed from reopened to closed

While I think it would be great to get the a11y change in, and while it's a very small change, I'm concerned that we haven't had enough testing with VoiceOver to be confident in this change so late in the cycle.

I'm going to close this as fixed again and open a new ticket with @afercia's patch. Once we spend some more time testing, I think it can go into the next cycle and a minor release.

#94 @rianrietveld
3 years ago

Test results form the accessibility test team on Wp 4.5 plus patch 33301.voiceover.diff

Same questions where asked as in the first test https://make.wordpress.org/accessibility/2016/02/25/accessibility-usertest-create-and-edit-links-inline/. Conclusion is at the bottom, only Gabriela tested with VoceOver.

Results:

Reagan Lynch (screen reader user & developer) This works great. I understand the instructions, I can edit the link, insert the link, and also look for a page or post to link to. When I type a word into the search field JAWS correctly reports that there are results and I can arrow through the results using the up and down arrows. Very nicely done. AT used: Windows 7 and JAWS 17

Shaun Everiss (screen reader user) Ok works. I didn't find any issues, access, incerting, searching for things to insert it works. AT used: NVDA, Windows 7, Firefox 45

Tobias Clemens Häcker (screen reader user & developer)

Q: Can you understand how to add a link using this inline window A: Yes and no. After pressing Ctrl + K I can start typing the URL or a search term and get voice notifications of the amount of results. However when using the up/down arrows to select the a single result only their number (6 of 20 for example) is spoken by NVDA, not the title. I can activate apply once it’s selected by hitting Tab to access the “Apply” button and then Enter to activate it. Q: Is this usable with your assistive technology? A: Yes and no, I don’t know which post is selected but typing in a URL works fine.

Q: Can you hear the search result if you enter a search word instead of an URL A: No, only the number of results or the number of a specific result is spoken.

Q: Are you missing functionality to make this work for you A: I miss voice output of search results.

AT used: NVDA, Windows 7, Chrome browser

Michelle DeYoung (developer)

Results Windows10/Chrome/NVDA: Can you understand how to search for a page using a search word using this inline window - Partially. Is this usable with your assistive technology? - Not the Search feature. If you use a screen reader: can you hear the search result if you enter a search word instead of an URL - No. The Search result list items are announced as "unknown", so user will not know what to select.

Results Win10/FF/NVDA The pop-up is announced as "Alert". The heading is not voiced by the screen readers. The heading should be announced as "Insert/Edit Link". The keyboard focus is dropped in the URL text field, so when the user tabs the first item they hear announced after 'Alert" is "Link text". If the user happened to arrow as their first movement in the popup, they are thrown down to the section list of content items. Each content item is not voiced, but their URL is - and the cursor focus remains in the URL field. The URL field is dynamically updated with the url of the items they are arrowing through in the list. The user will not know that the URL is being updated dynamically. I suggest it gets updated only when a selection is made. If the URL field was initially populated with a user URL, that URL should remain until they have decided to make another selection. The section list of content is announced as "Section". I think it needs to be clearer about what the section contains. When the user tabs to the Section list of content, they can arrow to the items, but none of them are announced.

Geof Collis (screen reader user & developer) I'm not sure how the wheel part works but I had no problems inserting a link or finding a page on the site. AT used: Windows 7, JAWS 14.5, IE 11.0.96

Sirisha Gubba (developer) Q: Can you understand how to add a link using this inline window? A: Instructions are given through place holder and as user starts typing in they are gone. AT user does not understand the if the search result is a page or the modified date next to it etc. Wheel should have more description then just "Link Option" button. Unable to close the "type/search URL" small window with "ESC" key

Q: Can you understand how to search for a page using a search word using this inline window? A: AT and Keyboard: Able to search it but unable to close the "Edit/delete" small window with ESC key and also user is unable to access the "EDIT" and Delete" button

AT used: NVDA, with FF on Windows7

Gabriela Nino de Rivera Torres (developer) Keyboard only test: Everything worked great. The only issue that I found is that the Esc key won't close the small window.

Safari/VoiceOver test: Can you access the little window with the input field for the link? - yes Can you understand how to add a link using this inline window? – yes Can you understand how to search for a page using a search word using this inline window? cmd +K opens the small window, VoiceOver announce it as a combo box and instructs me to type text to display a list of choices. Is this usable with your assistive technology? I will say yes, but some details need attention:

1) When opening the small window with cmd + K, the focus will jump very fast two or three times before settling down on the input field. I hear VoiceOver announcing: “li, li, links selected”. This problem won’t prevent the user to use the functionality because once the focus is on the input field it will stay there.

2) When searching on the input field, I see the number of results found on the VoiceOver text console, but there is not enough time to announce them before VoiceOver announce me the word that I typed on the input field. I tried lowering VoiceOver speech rate and pitch, also I changed the verbosity from high to medium. It did a little difference, but in most of the cases the results are not announced.

3) I’m not sure if the extra options window is part of the test, but when searching, VoiceOver does not announce the total of results found. If I move to the list of the results, VoiceOver won’t announce the result when it receives focus. Although, it does announce them in the small window (like Twitter does ;-)).

If you use a screen reader: can you hear the search result if you enter a search word instead of an URL? See comment number 2 below. Are you missing functionality to make this work for you? I won’t say missing a functionality since only small details need to be corrected to make it work with the assistive technology.

Conclusions Common remark: the window doesn't close on Esc. The functionality works understandable in IE/JAWS, FF/NVDA Chrome/NVDA has isses: it misses voice output of search results. For Safari/Voiceover: see remarks 1,2 and 3 from Gabriela.

This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.