aperezdc / ngx-fancyindex

Fancy indexes module for the Nginx web server
Other
849 stars 128 forks source link

Draft: Add table sort classes and aria-sort attribute #142

Open ryandesign opened 2 years ago

ryandesign commented 2 years ago

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 table tag: 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-type only 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 class link. If it were up to me I would change class link everywhere to be class name. 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, then nth-of-type wouldn'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-sort attribute 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 the th tag, it cannot be used to style the corresponding td tags, so I still added the classes to the table tag as well.

Let me know if you have thoughts or concerns about any of the above or about what I've committed so far.