WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 17 months ago

Last modified 17 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 17 months 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 17 months ago.
Slightly shift "Configure" link in Dashboard widgets
22456-post-and-quickpress-labels.diff (2.8 KB) - added by lessbloat 17 months ago.
22456-items-3-5.diff (433 bytes) - added by lessbloat 17 months ago.
22456.diff (9.6 KB) - added by lessbloat 17 months ago.
22456.2.diff (9.6 KB) - added by lessbloat 17 months ago.
22456.3.patch (10.5 KB) - added by ocean90 17 months ago.
22456.4.patch (4.4 KB) - added by lessbloat 17 months ago.
22456.5.patch (3.5 KB) - added by lessbloat 17 months ago.
22456.5b.patch (3.6 KB) - added by lessbloat 17 months ago.

Download all attachments as: .zip

Change History (30)

comment:1 DrewAPicture17 months ago

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

comment:2 lessbloat17 months ago

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

1) Metaboxes throughout the admin show expand/collapse arrows and draggable hand cursors when neither is possible without JS.

2) Select all checkboxes in tables should be hidden.

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

4) The featured image metabox is showing, but empty: http://cl.ly/image/460Q2O2E1H0E

5) Clicking the "Add Custom Field" button on post-new.php reloads the page, but does nothing.

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

Version 0, edited 17 months ago by lessbloat (next)

comment:3 TobiasBg17 months 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.

TobiasBg17 months ago

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

comment:4 TobiasBg17 months 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.

TobiasBg17 months ago

Slightly shift "Configure" link in Dashboard widgets

comment:5 follow-up: lessbloat17 months 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

lessbloat17 months ago

comment:6 lessbloat17 months ago

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

comment:7 in reply to: ↑ 5 ; follow-up: ocean9017 months 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

lessbloat17 months ago

comment:8 in reply to: ↑ 7 lessbloat17 months 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 DrewAPicture17 months ago

  • Cc xoodrew@… added

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

comment:10 follow-up: nacin17 months ago

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

lessbloat17 months ago

comment:11 in reply to: ↑ 10 lessbloat17 months 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.

ocean9017 months ago

comment:12 ocean9017 months ago

  • Keywords has-patch commit added

22456.3.patch​:

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

lessbloat17 months ago

comment:13 follow-up: lessbloat17 months ago

22456.4.patch uses CSS to hide check all checkboxes.

lessbloat17 months ago

comment:14 lessbloat17 months 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: TobiasBg17 months ago

Replying to lessbloat:

22456.4.patch uses CSS to hide check all checkboxes.

Using the existing .hide-if-no-js class 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.

Last edited 17 months ago by TobiasBg (previous) (diff)

lessbloat17 months ago

comment:16 lessbloat17 months ago

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

comment:17 in reply to: ↑ 15 helenyhou17 months 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 nacin17 months 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 ocean9017 months ago

#19662 was marked as a duplicate.

comment:20 nacin17 months 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.
Note: See TracTickets for help on using tickets.