Make WordPress Core

Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#28873 closed defect (bug) (fixed)

JavaScript code for adding bookmarklet Press This is hard to access with keyboard only

Reported by: rianrietveld's profile rianrietveld Owned by:
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.8
Component: Press This Keywords: has-patch
Focuses: accessibility Cc:

Description

In Tools ( /wp-admin/tools.php ) you have to click right on "Press this" to see the Javascript for adding the bookmarklet Press This if you can’t drag-drop.
Right click is complicated when you use only a keyboard on a Mac without using Voice Over.
Why not show the code by default or provide an easier way to hide/display the textarea.

Attachments (9)

28873.patch (1.3 KB) - added by joedolson 9 years ago.
Show press this button code
28873.2.patch (3.3 KB) - added by afercia 9 years ago.
With "Copy" button.
28873.3.patch (3.4 KB) - added by azaozz 9 years ago.
28873.4.patch (4.1 KB) - added by afercia 9 years ago.
28873.5.patch (8.5 KB) - added by afercia 9 years ago.
28873.6.patch (2.3 KB) - added by afercia 9 years ago.
minor text improvements and apostrophe
28873.7.patch (2.3 KB) - added by afercia 9 years ago.
28873.8.patch (2.3 KB) - added by afercia 9 years ago.
28873.9.patch (2.3 KB) - added by afercia 9 years ago.

Download all attachments as: .zip

Change History (35)

#1 @SergeyBiryukov
10 years ago

  • Component changed from Administration to Press This

#2 @helen
10 years ago

  • Version changed from trunk to 3.8

#3 @joedolson
9 years ago

  • Keywords has-patch dev-feedback added

The code is already present in a hidden textarea; but assigning display none makes it unavailable. All this patch does is remove the style attribute so that the field is available by default.

It's also an option to attach some basic show/hide toggling to this, but the page is hardly crowded, so it's not a priority. However, showing the code is on the ugly side!

@joedolson
9 years ago

Show press this button code

#4 @afercia
9 years ago

What about a dedicated button?

https://cldup.com/ET0Ihch1B3.png

@afercia
9 years ago

With "Copy" button.

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


9 years ago

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


9 years ago

@azaozz
9 years ago

#7 @azaozz
9 years ago

Having a button that reveals the hidden textarea containing the bookmarklet code seems good for both touchscreen/mobile and keyboard accessibility.

In 28873.3.patch: cleaned up a bit 28873.2.patch, made it to slideToggle so it doesn't jump, and "closable" when the button is clicked again.

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

#8 @afercia
9 years ago

Tested patch .3 with Firefox + NVDA and IE 8 + JAWS 15, works and we can make it even better. I would propose a few changes. The main issue is about focus when closing the revealed panel:

  • in Firefox: close the panel, focus stays in place, tab again and focus goes to the "Categories and Tags Converter" link. That's fine.
  • in Chrome: close the panel, focus is lost, tab again and the tab sequence starts over from the body

I think that's because focus is moved to the sliding panel even when the animation slides up: when the animation is complete, the element is hidden and there's no element to focus anymore. Browser's behavior differs here and we know Firefox handles focus better than Chrome. The updated patch moves focus just when sliding down.

Added also aria states and properties, screen readers will now announce the expanded/collapsed status and the instructions provided. Used aria-labelledby instead of aria-describedby to make JAWS read out the instructions *before* the 3286 characters of code :) aria-describedby gets read out *after*.

Added 1 sec. timeout before moving focus to give screen readers the time to announce "expanded".

The following example is what NVDA reads out now:

Copy Press This code
button collapsed

[press Enter]

expanded
Press This code edit read only multi line
Copy the selected code below, open your Bookmarks manager, create new bookmark, type Press This into the name field and paste the code into the URL field.
selected 3286 characters

@afercia
9 years ago

#9 follow-up: @azaozz
9 years ago

28873.4.patch looks good. Couple of things:

  • If we need IDs for aria, we can remove the classes (don't see the pressthis-code-wrap ID used anywhere).
  • If the animation (slideToggle) is "messing up" with the keyboard navigation/accessibility, we can remove it. Usually "jumpy" UI is worse UX, but...

These changes also need to be added to options-writing.php. The Press This button/link is shown in two places :)

#10 in reply to: ↑ 9 @afercia
9 years ago

Replying to azaozz:

  • If we need IDs for aria, we can remove the classes

I've considered that but didn't want to change your code too much :) will do

  • If the animation (slideToggle) is "messing up" with the keyboard navigation/accessibility, we can remove it.

No issues with keyboard accessibility, just a little "jumpy" should be fixed now see CSS

These changes also need to be added to options-writing.php. The Press This button/link is shown in two places :)

Will do.

#11 @DrewAPicture
9 years ago

  • Priority changed from normal to high

@afercia
9 years ago

#12 @afercia
9 years ago

Updated patch to address @azaozz considerations.
Additionally:

  • didn't touch the button styles, there's ongoing discussion about this
  • don't see a good reason to move focus on the textarea, it causes more problems than it helps solve
  • the bookmarklet code now gets selected on focus and on click
  • the aria-expanded attribute needs to be changed immediately, not after animation completes (noticed ChromeVox won't get the attribute value changed otherwise)

Edit:
we can't change the "Press This" link text because it will be used as bookmark name.

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

#13 @stephdau
9 years ago

How are we doing, now that #31373 (revamped Press This) has been merged? We've taken this ticket in consideration when developing the new version and screens.

#14 @afercia
9 years ago

Hi @stephdau thanks. In the revamped tools page I'd just change two very small details to make text more clear when read out of context and address the "apostrophe" issue in translatable string, e.g. device’s bookmarks instead of device\'s bookmarks. See attached new patch. Then I'd say this could be closed and new issues reported in the main ticket: #31373 or new tickets if there are no objections.

@afercia
9 years ago

minor text improvements and apostrophe

@afercia
9 years ago

#15 @afercia
9 years ago

Updated patch, fixing English grammar.

#16 @stephdau
9 years ago

Thanks afercia. :)

#17 @helen
9 years ago

  • Keywords needs-refresh added; dev-feedback removed

@afercia Patch seems to need a refresh, also take a look at the double quotes while you're in there :) “press”.

@afercia
9 years ago

#18 @afercia
9 years ago

Refreshed :)

#19 follow-up: @helen
9 years ago

Should Copy Press This Bookmarklet's code be Copy Press This Bookmarklet Code? Doesn't really need the possessive, and the mixed capitalization feels weird.

#20 in reply to: ↑ 19 ; follow-up: @marcelomazza
9 years ago

Replying to helen:

Should Copy Press This Bookmarklet's code be Copy Press This Bookmarklet Code? Doesn't really need the possessive, and the mixed capitalization feels weird.

Speaking about that, someone sometime mentioned quoting both Press This words, to avoid reading the phrase as "Copy Press, This Bookmarklet Code". I think the suggestion was:

Copy "Press This" Bookmarklet Code

#21 in reply to: ↑ 20 @stephdau
9 years ago

Replying to marcelomazza:

"Copy Press, This Bookmarklet Code"

That was me, and I find the quoted version better.

#22 @afercia
9 years ago

Yup capitalization is mixed, sorry for that. Thinking maybe there's a bit too much capitalization in WordPress :) and maybe I'm not alone with this feeling. For example, things like this:

Log Out of All Other Sessions

are a bit too much for me :) Anyway, please let me know about quotes, I would do it this way:

copy "Press This" bookmarklet code

@afercia
9 years ago

#23 @afercia
9 years ago

Refreshed :) Worth nothing that string is now used as a screen-reader-text so is not visible.

#24 @azaozz
9 years ago

In 31768:

PressThis: improve translatable strings on the Tools screen.
Props: afercia. See #28873.

#25 @azaozz
9 years ago

  • Keywords needs-refresh removed
  • Milestone changed from Awaiting Review to 4.2
  • Resolution set to fixed
  • Status changed from new to closed

I believe this is fixed now. Feel free to reopen if revealing the bookmarklet JS is still a problem.

#26 @DrewAPicture
9 years ago

  • Priority changed from high to normal
Note: See TracTickets for help on using tickets.