WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 6 months ago

#27314 closed enhancement (fixed)

Make sure button-primary has button class

Reported by: iseulde Owned by: chris_d2d
Milestone: 4.7 Priority: normal
Severity: normal Version: 3.8
Component: General Keywords: has-patch
Focuses: ui, administration Cc:

Description

This patch:

  1. Adds the button class to all primary buttons. This is necessary to make them responsive.
  2. Removes all secondary-buttonclasses and replaces it with button when needed. button-seconarydoes the same as button, but it's not responsive. It shouldn't be used.
  3. Replaces classes inside submit_button() with a shorthand for some buttons, a shorthand is used most of the time.

Attachments (4)

27314.patch (29.2 KB) - added by iseulde 3 years ago.
27314-2.patch (10.5 KB) - added by dimchik 16 months ago.
Updated the patch to incorporate the latest changes to the related files.
27314-3.patch (13.1 KB) - added by chris_d2d 16 months ago.
27314.diff (65.1 KB) - added by afercia 6 months ago.

Download all attachments as: .zip

Change History (21)

@iseulde
3 years ago

#1 @iseulde
3 years ago

  • Focuses ui administration added
  • Keywords has-patch added

#2 @johnbillion
3 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement
  • Version changed from trunk to 3.8

#3 @DrewAPicture
2 years ago

  • Keywords needs-refresh ui-feedback added

Patch needs a refresh and feedback from the UI folks.

#4 @chriscct7
16 months ago

  • Keywords good-first-bug added

#5 @chris_d2d
16 months ago

I'm taking a look at this for #wcus-contributor-day, let me know in slack (chrissmith) if you want to collaborate or are already working on this.

@dimchik
16 months ago

Updated the patch to incorporate the latest changes to the related files.

@chris_d2d
16 months ago

#6 @chris_d2d
16 months ago

  • Keywords needs-testing added; needs-refresh removed

I found a couple more locations where button-secondary could be removed:
src/wp-includes/customize/class-wp-customize-nav-menu-control.php
src/wp-includes/customize/class-wp-customize-new-menu-section.php
src/wp-includes/customize/class-wp-widget-area-customize-control.php

These are related to #19910's move away from switch statements for the customizer.

#7 @helen
16 months ago

Widgets in the customizer is using .button-secondary in the JS, that needs to be considered here. We could probably use some documentation somewhere about what the classes are and what remains as back-compat. That can just be in this ticket for now while we figure it out.

#8 @DrewAPicture
10 months ago

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

Assigning to mark the good-first-bug as "claimed".

#10 @afercia
8 months ago

Related: #37138

Last edited 8 months ago by afercia (previous) (diff)

#11 @afercia
7 months ago

  • Milestone changed from Future Release to 4.7

It would be nice to fix this for 4.7 together with #37138 and #37448 🙂

#12 @afercia
6 months ago

In 38616:

Accessibility: Standardize the remove/delete/cancel links in the Menus screen and Publish meta boxes.

The submitdelete CSS class is used in various places across the admin for some
"red" action links. It is worth simplifying this rule for further improvements
related to color contrast.

  • simplifies a non-standard styling for the "Remove/Cancel" links in the Menus screen
  • underlines all the "Move to trash/Delete" red links in all the Publish meta boxes
  • fixes CSS classes usage for all the Publish meta boxes primary buttons
  • fixes broken layout for the old Link Manager publish meta box

Props karmatosed, hugobaeta, monikarao, afercia.
Fixes #37969, #37018. See #37448, #37138, #27314.

@afercia
6 months ago

#13 @afercia
6 months ago

  • Keywords ui-feedback good-first-bug needs-testing removed

Refreshed patch. Tried to address all the points mentioned in the ticket description and comments. Additionally, there's a mixed situation now where submit_button() is sometimes used passing an empty string for the CSS class, and sometimes passing button (alone or together with other classes). Since passing button is unnecessary, I'd propose to standardise and remove it. This implies a small adjustment to get_submit_button() in order to remove empty items to avoid a single class button being printed out with a space at the end class="button ".

The patch is pretty big and would need to be merged as soon as possible :) Any feedback welcome, see also the QUnit tests part. /cc @helen @iseulde

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


6 months ago

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


6 months ago

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


6 months ago

#17 @afercia
6 months ago

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

In 38672:

Administration: Improve the usage of the button CSS classes.

Introduces some consistency in the usage of the button CSS classes, fixes the
focus style for accessibility and responsiveness of the buttons.

  • Adds the button class to all primary buttons make them responsive
  • Removes all secondary-button classes and replaces it with button when needed. button-secondary shouldn't be used and exists just for backward compatibility reasons
  • Replaces classes inside submit_button() with a shorthand for some buttons, and use an empty string for the default button class. Passing button is unnecessary
  • Adjusts get_submit_button() to remove empty items

Props iseulde, dimchik, chris_d2d, mhowell, afercia.
Fixes #27314, #37138, #37448.

Note: See TracTickets for help on using tickets.