-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Filter sort #117
Filter sort #117
Conversation
@Heydon, I feel conflicted about this PR. Can you help resolve that? |
@micmath I'll attempt a rebase. Hold onto your hat! |
@micmath I can easily fix the conflicts, but I noticed that merging would delete the tables component. I'm not sure why this would be (or what other work would be lost) at the moment. |
@micmath Okay, I've fixed this. Sorry it's now such a big PR. I can try squashing if you need. The only relevant files for review are the ones relating to Filter/Sort. |
I’ll have a go, but you may need to go back after and fix a few things. |
@micmath Nothing should break, since the search, tables, and infographics stuff are already merged in. But it's a lot in one place, sorry. |
You are right there weren’t any merge conflicts. I made some changes (see below) so you will need to Playing around with the filter demo on the actual documentation page, I noticed that pressing a several filter tabs in series was not a great experience: clicking The actual answer is that we shouldn’t be reloading whenever the filter is changed, in my opinion. Ideally that would involve a spinner and a |
@micmath Ah, yes. Some demos don't really suit the inline/embedded versions. I did my best to emulate the behaviour of the Academy search, which I liked because it didn't require JavaScript. If JavaScript was involved, we wouldn't want to use links. It would be a form and radio buttons, most likely. The filter constructor JS only adds the 'More' menu enhancement. The load more functionality would use JavaScript, but only for successive clicks of 'Load more' after the initial filter/sort endpoint is loaded. That just leaves the bespoke JS to emulate whole page loads in the single page demo. |
I think a better example to look at is Food. For example a search for chocolate can be filtered in many ways. Ignoring the oddity of having menus in the filter tabs, they do manage to keep keyboard focus on the filter control element, even after the control is activated. So, for example I can tab over to the “Dairy-free” filter option, activate it (thus triggering the result to be updated in page), and then just keep tabbing along from there. If JS is available, can the demo just update the content in the area that shows the results, without reloading the whole page please? |
Thinks |
If that sounds like a donkey’s job in JS, I can break out my code editor and give some help. I expect @simonsinclair could lend a hand too. |
@micmath I was only aware of the Academy pattern, and implemented since I liked it that way (although I fixed the fact that the sorting was not at all keyboard accessible). What you are proposing is a completely different, and more complex, pattern (I think). The Food implementation is broken without JavaScript (the filters appear but do nothing). It might be possible to progressively enhance the links-based version I have already done to create radio button semantics (ARIA) and behaviors. Thoughts? |
I must not have explained what I was asking for properly, because if I had there is no way you would say it was "completely different, and more complex". How did you fix the fact that the sorting was not at all keyboard accessible by the way? I don't see that change in github yet. My suggestion was/is basically that filters work like an asynchronous version of Tabs. So, just like Tabs, the page shouldn't reload upon a tab (read "filter") being selected. In fact you can ignore the asynchronicity aspect too and leave that as an exercise for the library implementor, I just thought that might be a fun/interesting nut to document. |
I did it by not trying to emulate the inaccessible dropdown at all. Instead, the two links appear side-by-side, with the 'current' one highlighted via Looking at the main search page itself (at https://www.bbc.co.uk/search), it appears to only expose filters with JS enabled. However, the pattern is in accordance with my implementation: it uses links, and whole page reloads, to access each filter. I'm concerned that diverging from this pattern too much will mean creating an interface that isn't compatible with the backend/API/architecture, or whatever's there. But maybe I shouldn't worry about that? Another thing I noticed is that the focus order has been augmented, so that the search field is first to be focused, before the skip links even. This I would recommend against, but was probably instated to improve full-page-load filtering by reducing the number of tabs to the search functionality. Your proposed JS-based solution would, as you said, address this — because using XHR would allow us to just change the contents, and keep focus on the current button. If we were to build this version on top of a basic link-based version, it would be necessarily more complex. It's possibly doable, but I'd probably have to enhance the links into ARIA radio buttons, which would mean supporting arrow key selection as well. Doing this across the principle links and 'more menu' would probably be a very confusing experience, since a number of the radio controls would be hidden behind the "More" button. There's also the concern that, without looking like radio buttons, sighted keyboard users would not be aware of the expected interaction paradigm. It would also be tricky to implement: would pressing the right arrow key on the right-most principle link both open the "More" menu and focus the first of its controls? Perhaps enhancing to basic buttons, and using In any case, all of this would be to save the user a few tab stops for successive filter/sorting selections. Not an inconsiderable improvement in terms of convenience, I agree, but I think the simplicity of the link-based interaction paradigm is both more clear, and more robust. Not forgetting that a page load automatically means the Whew. Sorry for the long comment. Those are all my thoughts so far! |
@micmath All yours! |
Includes the search component originally in #109 and already reviewed