WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 4 months ago

#22456 closed defect (bug) (fixed)

Audit the admin with JavaScript off

Reported by: nacin Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Accessibility Keywords: audit has-patch commit
Focuses: Cc:

Description

This 3.5 pre-release task is to double-check new and existing features, screens, etc., that things look sane with JavaScript off, that JavaScript-dependent features are not shown, etc.

lessbloat indicated he would look at this. Would be great if someone else can take it for a spin as well.

Attachments (10)

22456-select-all-checkboxes.diff (6.9 KB) - added by TobiasBg 2 years ago.
Add "hide-if-no-js" to all select all checkboxes in WP_List_Tables
22456-dashboard-widget-configure.diff (1.0 KB) - added by TobiasBg 2 years ago.
Slightly shift "Configure" link in Dashboard widgets
22456-post-and-quickpress-labels.diff (2.8 KB) - added by lessbloat 2 years ago.
22456-items-3-5.diff (433 bytes) - added by lessbloat 2 years ago.
22456.diff (9.6 KB) - added by lessbloat 2 years ago.
22456.2.diff (9.6 KB) - added by lessbloat 2 years ago.
22456.3.patch (10.5 KB) - added by ocean90 2 years ago.
22456.4.patch (4.4 KB) - added by lessbloat 2 years ago.
22456.5.patch (3.5 KB) - added by lessbloat 2 years ago.
22456.5b.patch (3.6 KB) - added by lessbloat 2 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 @DrewAPicture2 years ago

As mentioned on make/core, there are some no-js suggestions on #18900

comment:2 @lessbloat2 years ago

UPDATE: I amended some of my findings.

Here are my findings (though, I'm not sure how much of this is directly related to 3.5 changes):

1) Select all checkboxes in tables should be hidden.

2) Post and page title/content lack labels: http://cl.ly/image/0p163c0E080j

3) The featured image metabox is showing a button, but it does nothing.

4) Clicking the "Enter new" link under custom fields on post-new.php does nothing.

5) Media Library -> Edit -> "Edit Image" button is showing, but does nothing.

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

comment:3 @TobiasBg2 years ago

I also noticed lessbloat's no. 1). Patch 22456-select-all-checkboxes.diff adds "hide-if-no-js" to all select all checkboxes in all WP_List_Tables. Now, as the select all checkboxes are rewritten for screen-reader-ness (see [21317]), the change in wp-admin/includes/class-wp-list-table.php would actually sufficient to add "hide-if-no-js". I opted to add it to all of them, regardless, to make things complete. The patch also removes an unnecessary global $status; in one of the files.

@TobiasBg2 years ago

Add "hide-if-no-js" to all select all checkboxes in WP_List_Tables

comment:4 @TobiasBg2 years ago

6) Now, with #21611 fixed, the "Configure" links that appear when hovering the "Recent Comments" or "Incoming Links" Dashboard widgets, could move a little bit to the right (or left in RTL) in non-JS mode. Patch 22456-dashboard-widget-configure.diff does this.

@TobiasBg2 years ago

Slightly shift "Configure" link in Dashboard widgets

comment:5 follow-up: @lessbloat2 years ago

In 22456-post-and-quickpress-labels.diff​ I added:

No-js labels for Quickpress:

http://f.cl.ly/items/3A221u0x0M2q0C35360S/quickpress-labels.jpg

No-js labels for add post/page (and removed the big gap between title and the content block):

http://f.cl.ly/items/3r1r0L1L2v3Z3f072R1Y/new-post-nojs-labels.jpg

@lessbloat2 years ago

comment:6 @lessbloat2 years ago

22456-items-3-5.diff​ hides items 3, 4, and 5.

comment:7 in reply to: ↑ 5 ; follow-up: @ocean902 years ago

Replying to lessbloat:

In 22456-post-and-quickpress-labels.diff​ I added:

No-js labels for Quickpress:

QuickPress should be hidden completely, see #19662/#13516.

Related: #18900

@lessbloat2 years ago

comment:8 in reply to: ↑ 7 @lessbloat2 years ago

Replying to ocean90:

QuickPress should be hidden completely, see #19662/#13516.
Related: #18900

I'm happy to remove QuickPress for non-js if that's the consensus. In 22456.diff​ I simply combined the following:

  • 22456-select-all-checkboxes.diff​
  • 22456-dashboard-widget-configure.diff​
  • 22456-post-and-quickpress-labels.diff
  • 22456-items-3-5.diff

comment:9 @DrewAPicture2 years ago

  • Cc xoodrew@… added

Related #22461, #22509 (Default background/header text color labels)

comment:10 follow-up: @nacin2 years ago

.no-js #dashboard_quick_press { display: none } can probably save us from work here. It doesn't work; it should be hidden.

@lessbloat2 years ago

comment:11 in reply to: ↑ 10 @lessbloat2 years ago

Replying to nacin:

.no-js #dashboard_quick_press { display: none } can probably save us from work here. It doesn't work; it should be hidden.

Done in 22456.2.diff.

@ocean902 years ago

comment:12 @ocean902 years ago

  • Keywords has-patch commit added

22456.3.patch​:

  • RTL support
  • fixes spaces vs tabs in wp-admin.css

@lessbloat2 years ago

comment:13 follow-up: @lessbloat2 years ago

22456.4.patch uses CSS to hide check all checkboxes.

@lessbloat2 years ago

comment:14 @lessbloat2 years ago

For 22456.5.patch​ I:

  • removed "Content" <label>(potential can of worms)
  • unhid featured image container
  • Hid featured image set and remove
  • consolidated some .no-js code

comment:15 in reply to: ↑ 13 ; follow-up: @TobiasBg2 years ago

Replying to lessbloat:

22456.4.patch uses CSS to hide check all checkboxes.

Using the existing .hide-if-no-js has the advantage that it adds "meaning" or "reasoning" as to why the checkboxes are hidden - directly to the checkboxes' markup. Using some extra CSS for this instead might make the markup shorter, but opens the question: "Why do we actually have and keep .hide-if-no-js then?"
The new CSS adds what .hide-if-no-js wants to prevent: Extra and hard to maintain CSS code.

Version 0, edited 2 years ago by TobiasBg (next)

@lessbloat2 years ago

comment:16 @lessbloat2 years ago

22456.5b.patch​ adds !important else .remove display is reset further down in CSS.

comment:17 in reply to: ↑ 15 @helenyhou2 years ago

Replying to TobiasBg:

Using CSS is semantically more correct - there's no reason the checkbox in .check-column will ever need to show without JS, because it's only functional via JS. Thus the practical advantage of the CSS is that it will work even if you do a custom column that uses the cb callback. It's not uncommon to see code like:

function my_cpt_edit_columns( $columns ) {
	$columns = array(
		'cb' => '<input type="checkbox" />',
		'title' => 'Name',
	);

	return $columns;
}

If you use .hide-if-no-js, then devs have to add that class to their markup as well, even if using the core-defined column callback (which is where the .check-column class comes from). Also, the updates screen uses these select all checkboxes, which were not accounted for in earlier patches, but are taken care of with this method.

comment:18 @nacin2 years ago

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

In 22719:

No-JS fixes.

  • Hide QuickPress.
  • Hide the gap between the title and content editor.
  • Hide 'select all' checkboxes in list tables.
  • Hide a non-functioning link in the custom fields metabox.
  • Hide non-functioning buttons in the featured image box (same as in 3.4).
  • RTL and miscellany.

props TobiasBg, lessbloat, ocean90. fixes #22456.

comment:19 @ocean902 years ago

#19662 was marked as a duplicate.

comment:20 @nacin2 years ago

If you use .hide-if-no-js, then devs have to add that class to their markup as well, even if using the core-defined column callback (which is where the .check-column class comes from). Also, the updates screen uses these select all checkboxes, which were not accounted for in earlier patches, but are taken care of with this method.

It's also worth pointing out a few things here:

  • It is hypothetically possible to implement "check all" server side, but no one sane would probably do that (for both technical and UX reasons), so this seems fine and good that it carries to other forms.
  • The only <input type="checkbox"> that would have needed to be modified is the one in WP_List_Table. That one overrides all over 'cb' columns to add some accessibility pieces. The remaining '<input type="checkbox" />' strings are placeholders as of 3.5.
  • .hide-if-no-js is a nice tool, yes, and it has its uses. But it doesn't beat well-organized CSS. You'll note that in [22719] I took pains to group pieces of CSS with their sections (which isn't what the patch you reviewed did). That makes it very obvious what is going on — and is a better heads-up when you are editing CSS, making it easier to maintain.

comment:21 @chriscct74 months ago

#13516 was marked as a duplicate.

Note: See TracTickets for help on using tickets.