Make WordPress Core

Opened 10 years ago

Closed 7 years ago

#27314 closed enhancement (fixed)

Make sure button-primary has button class

Reported by: iseulde's profile iseulde Owned by: chris_d2d's profile 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 10 years ago.
27314-2.patch (10.5 KB) - added by dimchik 8 years ago.
Updated the patch to incorporate the latest changes to the related files.
27314-3.patch (13.1 KB) - added by chris_d2d 8 years ago.
27314.diff (65.1 KB) - added by afercia 7 years ago.

Download all attachments as: .zip

Change History (21)

@iseulde
10 years ago

#1 @iseulde
10 years ago

  • Focuses ui administration added
  • Keywords has-patch added

#2 @johnbillion
10 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
9 years ago

  • Keywords needs-refresh ui-feedback added

Patch needs a refresh and feedback from the UI folks.

#4 @chriscct7
8 years ago

  • Keywords good-first-bug added

#5 @chris_d2d
8 years 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
8 years ago

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

@chris_d2d
8 years ago

#6 @chris_d2d
8 years 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
8 years 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
8 years ago

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

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

#10 @afercia
8 years ago

Related: #37138

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

#11 @afercia
8 years 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
8 years 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
7 years ago

#13 @afercia
7 years 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.


7 years ago

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


7 years ago

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


7 years ago

#17 @afercia
7 years 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.