WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#35126 closed enhancement (fixed)

Improvements for the new .button-link CSS class

Reported by: afercia Owned by: Cheffheid
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.4
Component: Administration Keywords: semantic-buttons has-patch
Focuses: ui, accessibility Cc:

Description

Splitting this out from #31476.

The .button-link CSS rule for link-like buttons introduced in [35636] provides a basic reset and just some basic styling.

A good question was raised on #31476, wondering if the .button-link CSS class should provide also the default/hover/active/focus colors and underline to actually look like real links.
Not doing that will force to implement them in every place the link style is used.

cc @helen, @michaelarestad

Attachments (8)

35126.patch (1.1 KB) - added by Cheffheid 3 years ago.
Takes the "extend what we have" approach, rather than "adjust the default and update what's affected" one.
35126.2.patch (3.1 KB) - added by Cheffheid 3 years ago.
Also adding the "adjust the default and update affected" route
35126.3.patch (4.6 KB) - added by Cheffheid 3 years ago.
Added .button-link to a number of places that remove text-decoration.
35126.4.patch (2.0 KB) - added by Cheffheid 2 years ago.
Adds two modifier classes for button-link: like-core-anchor (blue), like-delete (red)
35126.diff (17.4 KB) - added by afercia 2 years ago.
35126.2.diff (17.6 KB) - added by afercia 2 years ago.
35126-ie11-postbox-underline.patch (316 bytes) - added by Cheffheid 2 years ago.
Fixes stray underline in IE11 on postbox toggle arrows
35126.3.diff (2.4 KB) - added by afercia 2 years ago.

Download all attachments as: .zip

Change History (57)

#1 @psconsole
3 years ago

@afercia Do you want to make the links to display like buttons?

http://i.imgur.com/8xgM6Lt.png

#2 @afercia
3 years ago

@psconsole hi, personally I'd make them look like buttons (not primary buttons though, just standard ones). By the way I didn't want to introduce visual changes since they should be thoroughly considered across all the admin. Open to suggestions from a UI perspective :)

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


3 years ago

#4 @afercia
3 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.5
  • Owner set to michaelarestad
  • Status changed from new to assigned


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


3 years ago

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


3 years ago

#7 @chriscct7
3 years ago

  • Milestone changed from 4.5 to Future Release

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


3 years ago

#9 @Cheffheid
3 years ago

#36553 was marked as a duplicate.

@Cheffheid
3 years ago

Takes the "extend what we have" approach, rather than "adjust the default and update what's affected" one.

#10 @afercia
3 years ago

  • Owner changed from michaelarestad to Cheffheid

@Cheffheid
3 years ago

Also adding the "adjust the default and update affected" route

#11 @Cheffheid
3 years ago

  • Keywords has-patch needs-testing 2nd-opinion added; needs-patch removed

Digging through everywhere the .button-link class is being used currently, and it looks like the impact of updating it is not that significant. For the most part, it results in underlines on modal close buttons, and a change in colour on the postbox triangles. Each of these should be fixed in patch 2, but should definitely have a second pair of eyes in case I missed something.

There are a few instances where I've left the change alone for further discussion/deciding:

http://i.imgur.com/IFknNhV.png
The Reorder link on Menus and Widgets in the Customizer became underlined. Brings it more in line with the other button links, and so I've left it as it is.

http://i.imgur.com/wuPfY1V.png
The Press This sidebar got a noticable hover state that it didn't have before. An improvement in its own right, though maybe not exactly in an agreeable style.

#12 follow-up: @afercia
3 years ago

  • Focuses accessibility added
  • Milestone changed from Future Release to 4.6

Personally, I like the direction here and I think this would make the .button-link class more useful and easier to use. A few things to consider, to improve the patch:

  • usually, links inside list tables are not underlined see .widefat a in common.css I'd suggest to do the same for .button-link when inside list tables
  • maybe worth searching all the CSS for text-decoration: none; to check if there are special cases that should be addressed
  • though I'd agree some styles should be consistent across all the WordPress ecosystem, I'm pretty sure the maintainers of Press This and the Customizer would like to keep the styles unchanged, since they're both sort of self-contained applications

Any thougths more than welcome. /cc @helen @michaelarestad @celloexpressions

#13 @celloexpressions
3 years ago

I'm fine with the added underline in the customizer, since that would match the delete buttons. I'm not sure if the button-link style is generally the best for this, but it provides nice hierarchy with the add buttons so we can keep it for now.

#14 @afercia
3 years ago

For reference, see also the GItHub implementation :) See the Save and Cancel buttons in the screenshot below:

https://cldup.com/qla18ZymSz.png

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


3 years ago

@Cheffheid
3 years ago

Added .button-link to a number of places that remove text-decoration.

#16 in reply to: ↑ 12 @Cheffheid
3 years ago

Replying to afercia:

  • maybe worth searching all the CSS for text-decoration: none; to check if there are special cases that should be addressed

I've added the .button-link class to where I thought it would make sense after doing this search. I may have been a little enthusiastic with adding it in places where I thought it could have buttons instead of links in the future though.

@celloexpressions Cool, thank you sir!

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


3 years ago

#18 follow-up: @jipmoors
3 years ago

While reviewing this patch I was missing the red color for the menu item remove link in the customizer.

<button type="button" class="button-link item-delete submitdelete deletion">Remove</button>

The .button-link.menu-delete has the red color, but the .button-link class adds a color to the remove link which displays as blue only now.

Reproduce
Opening the customizer
Open a menu (or create one)
Open a menuitem (or create one)
Compare the remove button/link to the trunk version.

#19 in reply to: ↑ 18 @afercia
3 years ago

Replying to jipmoors:

While reviewing this patch I was missing the red color for the menu item remove link in the customizer.

Yep, while it makes sense to me to have the default button-link color set to blue, then the ones that need to be red should use a rule with a higher specificity than .wp-core-ui .button-link. This probably needs some more thoughts.

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


3 years ago

#21 @afercia
3 years ago

  • Keywords early added
  • Milestone changed from 4.6 to Future Release

Punting to future release, hoping in a big help from the next release lead ;)

#22 @afercia
3 years ago

  • Keywords early removed
  • Milestone changed from Future Release to 4.7

This ticket was mentioned in Slack in #core by desrosj. View the logs.


3 years ago

#24 @desrosj
3 years ago

This just needs a consensus on the proposed direction, and then it should be good to go after more testing.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 years ago

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


3 years ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 years ago

#29 @Cheffheid
3 years ago

As per the accessibility meeting last Monday, we'll continue on the "extend what we have" path for this ticket. That way we can keep the .button-link class as it is and change the button-links that need to be blue to blue in a more controlled manner without having to worry about overriding the ones that have their style already mostly taken care of (like the red .menu-delete ones).

I'll refresh the first patch and append the new class to buttons that need it.

Anyone have any qualms about using .like-default as the class name for the "default blue link" style (trying to keep the class name colour agnostic, so that it can be overwritten for high-contrast admin themes and still make sense)?

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


3 years ago

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


2 years ago

@Cheffheid
2 years ago

Adds two modifier classes for button-link: like-core-anchor (blue), like-delete (red)

#32 @Cheffheid
2 years ago

Sorry about that.

Opted to go with .like-core-anchor as a modifier to turn a button link into something that resembles a core anchor more closely in most browsers. Except the ones that don't add an underline to anchor tags by default, since it does add that still.

Also added a .like-delete as a modifier for red links.

Some testing and review appreciated, feel free to tear it apart. :)

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


2 years ago

#34 @helen
2 years ago

  • Milestone changed from 4.7 to Future Release
  • Type changed from defect (bug) to enhancement

I would rather see this get a full base set of styles because of its name (we went with link and not reset, right?) and then work on fixing places that end up broken. This also seems more like an enhancement than a bug fix, so punting.

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


2 years ago

#36 @afercia
2 years ago

See also the last Slack conversation linked above for a possible future direction.

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

#37 @Cheffheid
2 years ago

The .2 patch still applies fine, so that'll be a good place to continue from then.

It already includes some fixes for button links that needed some additional love to restore their look as they were before, but I imagine there'll be more at this point (and some that manually had their styles updated to links that need to be cleaned up).

Wouldn't mind some help with going through all of that. :)

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

#38 @helen
2 years ago

In 39194:

Customize: Make "Add New Page" buttons less intrusive.

They now better match the existing add functionality from the category metabox and are better separated from the surrounding inputs. All hail .button-link.

fixes #38164. see #35126.

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


2 years ago

@afercia
2 years ago

#40 @afercia
2 years ago

  • Keywords needs-testing 2nd-opinion removed
  • Milestone changed from Future Release to 4.8

Refreshed patch builds on the previous ones, with some adjustments discussed with @Cheffheid.

Quoting the conversation on Slack linked above (2 links above), the consensus was about:

it's way simpler to just update ⁠⁠⁠⁠button-link⁠⁠⁠⁠ adding color and underline, then modifier classes for red links etc.

In 35126.diff the base class just makes a button look like a regular link and then it can use modifiers as stackable classes, in the same way all the WP buttons CSS classes work. For example, button-link button-link-delete makes a button link red.

In a few places, some button-links will be now underlined. I'd say it's not too bad and seems more consistent to me, see screenshots below. Special places to check:

  • Customizer
  • Media Views
  • Press This: I've removed some button-link classes since they were mainly used to reset the buttons.

Worth noting button-link was never intended as a "button reset" and shouldn't be used that way. There's probably the need for a button-reset specific class, but I'd say that's material for a separate ticket.

Moving to 4.8.

https://cldup.com/xKHexm5CH1.png

https://cldup.com/GOSTB0v3Ld.png

https://cldup.com/ZFqFa7wi1W.png

https://cldup.com/aXDwRnuQgY.png

https://cldup.com/Ym1WXtrOuY.png

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

@afercia
2 years ago

#41 @afercia
2 years ago

Refreshed patch to reset the subtle CSS transition on the media .wp-core-ui .attachment-close.

#42 @afercia
2 years ago

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

In 40052:

Buttons: Improve the .button-link CSS class for link-like buttons.

Updates .button-link adding color and underline to make link-like buttons look
like links by default. Introduces .button-link-delete as a modifier, stackable
CSS class for red button-links.

Props Cheffheid, afercia.

See #34242.
Fixes #35126.

#43 @afercia
2 years ago

  • Keywords needs-patch added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Minor visual breakages were expected. Noticed a couple so far:

In IE 11 (doesn't happen on modern browsers): the post boxes handlediv shows the underline

https://cldup.com/UqQOBXgkHI.png

In all browsers: the Customizer "reorder toggle" needs some love: the button contains some span elements that use their own colors so on some button states, text and underline have different colors:

https://cldup.com/D7Q8m7yaGz.png

#44 @Presskopp
2 years ago

Let's integrate #38712 occasionally

#45 @Cheffheid
2 years ago

It's slightly weird that IE11 would render that underline.

text-decoration is being set to none on the :before with an !important and everything, but I guess having .button-link set it to underline is enough for it to render it on the button's content.

Setting text-decoration to none on the button seems to fix it. I'll upload a patch for that.

@Cheffheid
2 years ago

Fixes stray underline in IE11 on postbox toggle arrows

#46 @afercia
2 years ago

  • Keywords semantic-buttons added

@afercia
2 years ago

#47 @afercia
2 years ago

  • Keywords has-patch added; needs-patch removed

I'd rather avoid encouraging the use of the .button-link class as a reset so I'd propose to remove it from the postboxes toggle.

About the Customizer .reorder-toggle, it used to have a dotted border but that doesn't apply any longer since the refactoring in [35636] which also removed the class .not-a-button. I'd say to just remove the rules specific to .reorder-toggle and let it use the colors from .button-link.

The new patch 35126.3.diff fixes also #38712, props to f.staude and Presskopp.

#48 @afercia
2 years ago

Tested the latest patch on IE 11 and Edge and I don't see stray underlines. The only thing, in IE 11 the Customizer menu items toggle icons are a bit off, but that happens also on 4.7 so I guess it's not related and should be addressed in a separate ticket:

https://cldup.com/zE3xC1g3DR.png

#49 @afercia
2 years ago

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

In 40059:

Buttons: Fix a few .button-link glitches after [40052].

Removes some stray underlines. Explicitly set the button text to be left aligned.

Props Cheffheid, f.staude, Presskopp.

See #34242.
Fixes #35126, #38712.

Note: See TracTickets for help on using tickets.