Skip to content
This repository has been archived by the owner on Sep 6, 2018. It is now read-only.

Send the row index into a rowFilter function #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rehno-lindeque
Copy link

@rehno-lindeque rehno-lindeque commented Jul 6, 2017

Since the sortable table performs its sorting internally it is not possible to do any client-side paging while maintaining the global sort order for all of the data in the table.

This PR addresses that by adding a rowFilter function which allows for any type of filtering, but specifically includes a sort index to be used for paging.

I slightly prefer a previous attempt where I modified the rowAttrs to take the sort index allowing one to set display: none, which is better for screen readers. This would also allow one to make dynamic changes to the styling based on the sort index of the row.

Unfortunately this required a change in the render function from lazy3 to lazy4 (which seems to be untenable?), or otherwise to completely change the schema being passed into the table (which seemed unwise).

In the absence of a lazy4 function I think that this is the simplest change that makes sense for introducing client-side paging. Please let me know what you think, any comments are welcome!

screenshot

This allows one to do client-side paging on a table while maintaining
the sort order given by the columns.
@ocharles
Copy link

I slightly prefer a previous attempt where I modified the rowAttrs to take the sort index allowing one to set display: none, which is better for screen readers.

Could you expand on why that is better for screen readers?

@rehno-lindeque
Copy link
Author

I think that a similar argument applies as to the hidden html5 attribute: https://stackoverflow.com/a/6708403/167485. It's not too relevant to the application we're working on, but I thought that someone else might care.

@ocharles
Copy link

I'm still misunderstanding exactly why you want display: none over not even having the element. What changes for a screen reader?

@rehno-lindeque
Copy link
Author

You'd use a class if you cared about accessibility, rather than inline display: none. I should have been more clear. Something like this:

.visually-hidden {
  display: none;
}

@media print {
  .visually-hidden {
    display: block;
  }
}

@media speech {
  .visually-hidden {
    display: block;
  }
}

Pretty useful for printing pages also.

Anyway, it really just boils down to the old presentation versus semantics argument, that one should not pin down the presentation directly in html (which is for content), but via styling instead.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants