Fix: accessible sort button in a list table#382
Conversation
|
Would this be better kept as a link if we change the sorting variables to sync with query vars, and link it to an actual URL like |
|
Yes but we will have to restructure the entire link logic, currently we have one link for sorting, we will have to create two links for each heading (ASC, DESC). I think it's an over kill. Besides, the sorting operation is not an external link, it's an action on a table on an existing page. I prefer to keep it as a button. |
louiswol94
left a comment
louiswol94
left a comment
There was a problem hiding this comment.
This PR is sound and merge-worthy as an accessibility and semantics fix. And real URLs as a larger follow-up.
| border: none; | ||
| background: none; | ||
| font: inherit; | ||
| color: #2271b1; |
There was a problem hiding this comment.
Are you sure about the hard coded color #2271b1?
There was a problem hiding this comment.
Yes, I'm sure. It's our $accent color. We use this color in multiple places.
There was a problem hiding this comment.
So why not use the varable $accent?
3 participants
Replace
<a href="#">with a<button>.