Draft: Add table sort classes and aria-sort attribute #142
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The quantity of template variables is about to increase and it's tedious to renumber all subsequent variables just to keep them in numerical order. One of the variables, t_parentdir_entry, already didn't have a leading number, so this change makes the rest of the variables consistent with that.
Add the aria-sort attribute to the th of the column by which the data is sorted. This is conceivably useful to let screen readers know how the table is sorted, but it also allows CSS styles to be applied, for example to highlight the sorted column header or to change an icon in the column header to indicate the sort direction. This required breaking the start of the table into separate variables so that the column headers can be addressed individually. This is a step toward being able to reorder (aperezdc#66) or remove (aperezdc#133) columns.
Add a class "sort-col-1", "sort-col-2", or "sort-col-3" to the table tag to indicate by which column the data is sorted, and a class "asc" or "desc" to indicate whether it is sorted in ascending or descending order. This is intended to be useful for applying a style to the sorted column or its header. Modify the default template to slightly darken the sorted column.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR works for me but I'm marking it as a draft because it may require more discussion before merging to decide the best approach.
I wanted to be able to apply a CSS style to the currently-sorted column but there didn't seem to be a way to do it (without JavaScript). This PR adds two classes to the
tabletag: one for sort criterion, one for sort direction/order. I also updated the template to use the sort criterion classes to shade the currently-sorted column a little darker.I debated for awhile about what specific class names to use for the sort criterion. I couldn't determine if there was an established standard for that sort of thing. For now it uses numbered classes (e.g.
sort-col-1) because that goes nicely with the way the CSS selector to target the column header has to be written (e.g..sort-col-1 th:nth-of-type(1)). However,nth-of-typeonly has to be used because the column headers don't have any classes. (This also came up in #133.) Ideally I would like to add the classes that are already used in the table cells to the column headers as well. That will result in a slight confusion where the header of the name column has the classlink. If it were up to me I would change classlinkeverywhere to be classname. That would be a breaking change, but if the plan is to release a new major version next, like 0.6.0 or even 1.0.0, a breaking change should be ok if communicated clearly in the release notes. (Making a breaking change after 1.0.0 is less desirable.) If the class were renamed and classes were applied to the column headers as well, thennth-of-typewouldn't be needed anymore and the sort column class names could be changed from numbered to named (e.g.sort-by-name) and the CSS selector to style the column could be simplified to.sort-by-name .name). Using named rather than numbered classes would be advantageous if there is a desire to apply styles to specific columns (for example, to align the size column to the right) and also to allow the order of the columns to be changed (requested in #66).In searching for a standard way to indicate table sort order I came across the
aria-sortattribute intended originally for screen readers but also addressable from CSS. Initially this seemed perfect, so I implemented it first, but then realized that since this attribute is only set on thethtag, it cannot be used to style the correspondingtdtags, so I still added the classes to thetabletag as well.Let me know if you have thoughts or concerns about any of the above or about what I've committed so far.