Skip to:
Content

Opened 3 years ago

Closed 3 years ago

#7211 closed enhancement (fixed)

Simplify markup in settings pane of edit group screen in wp-admin.

Reported by: dcavins Owned by: mercime
Milestone: 2.7 Priority: normal
Severity: normal Version: 2.6.1.1
Component: Groups Keywords: has-patch commit
Cc:

Description

While working on hierarchical groups, @mercime and I talked about the markup in the "Settings" pane of the group edit screen in WP Admin. At the moment, it adds an unordered list structure around the inputs and labels, which is not ideal for screen readers.

I looked at how WP structures similar input, like the "Post Format" meta box, and it uses <br>s to form a list. So I've updated the structure and styling for our meta box inputs to not change visually while using the simplified html structure. I've also replaced id selectors with class selectors where possible.

Attachments (6)

7211.01.patch (4.2 KB) - added by dcavins 3 years ago.
Remove unnecessary unordered list from settings panel. Compensate.
wp-admin-group-settings-before-after.png (16.1 KB) - added by dcavins 3 years ago.
Showing before and after changes.
7211.02.patch (5.7 KB) - added by mercime 3 years ago.
group-settings-mobile.png (9.9 KB) - added by mercime 3 years ago.
7211.03.patch (5.7 KB) - added by mercime 3 years ago.
admin-rtl.css.patch (450 bytes) - added by mercime 3 years ago.

Download all attachments as: .zip

Change History (15)

@dcavins
3 years ago

Remove unnecessary unordered list from settings panel. Compensate.

@dcavins
3 years ago

Showing before and after changes.

#1 @DJPaul
3 years ago

LGTM

#2 @mercime
3 years ago

Thanks for creating the ticket @dcavins :)

Attached is a patch which removes the <br /> tags as well and adds spaces between radio and checkbox input controls for mobile view.

@mercime
3 years ago

#3 @dcavins
3 years ago

  • Keywords commit added

I like making the <label>s a block element better than using <br>s, too. :)

Let's make sure to change the selector #bp_group_settings legend in admin-rtl.css to .bp-groups-settings-section legend? (Do we need to touch admin-rtl.css directly anyway? Isn't it generated via the RTL-CSS grunt task?)

#4 @mercime
3 years ago

Thanks @dcavins :) Yes, we do need to reflect style changes in admin-rtl.css directly as admin CSS is not covered in the RTL-CSS grunt task ... yet. Good spot on the selector. Will commit in a few.

@mercime
3 years ago

#5 @mercime
3 years ago

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

In 10985:

Groups: Improve markup of the input controls in the Group Edit screen.

Props dcavins, mercime.
Fixes #7211.

#6 @DJPaul
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#7 @DJPaul
3 years ago

@dcavins @mercime grunt commit does generate RTL CSS for src/bp-groups/admin/css/admin.css. Please never edit the RTL CSS directly. As a result of r10985, grunt commit thinks the RTL CSS has been entered incorrectly: https://gist.github.com/9fe60e61b09dc10f9f3d0d7563c7b815

If your testing has shown your change to be correct, then please edit the CSS file and set up a control directive to tell RTL CSS to not adjust it. Docs here: http://rtlcss.com/learn/usage-guide/control-directives/

Otherwise, we can run grunt commit and commit the suggested change.

Thanks!

#8 @mercime
3 years ago

Thank you @DJPaul, you're right as usual. My bad. Will commit corrections in a minute.

#9 @mercime
3 years ago

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

In 10987:

Fix error in admin-rtl.css due to [10985].

Props DJPaul.
Fixes #7211.

Note: See TracTickets for help on using tickets.