-
Notifications
You must be signed in to change notification settings - Fork 360
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
Adds refresh buttons to jumpMenu select boxes #2492
Conversation
…o prevent unexpected page reloads issue
I just noticed that
are also affected. Now, the easiest option would seem to get rid of the jumpMenu definition in common.js and remove it from the phtml files + add the buttons. What do you think? |
Thanks, @ckaz. I'm interested in hearing from @crhallberg on this too, but I have some initial thoughts. 1.) I see drop-down menus that have immediate effects in a lot of places -- for example, the "tools" on Google search results, and the sort control in Amazon results. Given that these are two of the most commonly used sites, I have to imagine that they've taken accessibility into consideration in their design. Is it possible that we can retain the functionality while making it more accessible? I'm concerned that putting buttons everywhere and crowding the user interface will reduce accessibility in other ways. If it's truly the only solution, I'm not entirely against it, but these major precedents make me think there might be a better way. 2.) I wonder if the work in #2378 might be relevant to this. That pull request creates a standard shared template for displaying drop-down menus with immediate effect. However, those menus, rather than auto-submitting the form, are actually just a collection of links, which seems likely to be a more accessible approach. Maybe one solution would be for us to refactor some or all of the jumpMenu select boxes to use this technique instead. I'm pretty sure it would be easily possible for most (though maybe not all). At very least, I think the search controls (sort/limit/etc.) would support this approach, and then we could avoid crowding the interface in this key area. If we have to add buttons in places where there's only one drop-down on the page, that's less of a concern. What do you think? |
I just checked the sort control on Amazon and it works differently from our sort control in that you can open the select box with the down arrow and choose your option. This is the kind of operability we would get after removing the jumpMenu function. It's the same with Google: I can choose there too. In VuFind, the up/down or left/right arrow picks an element which I don't even know exists and kicks me there. So, the suggestion of the accessibility testers was removing adding a button to make the choice clearer or adding explanatory text regarding the functionality of the box (which would still not give users a choice of option from the select box). You may wonder why I do I take their suggestions so seriously. The test group is based on the German National Libary for the Visually Impared so they are bound to have a very user-based perspective and some of the tests are run by non-sighted users.
I understand and share your concern here and I believe that a correctly funtioning select box where users can navigate between the choices could funktion well without an extra button, as on Google and Amazon. However, please see https://www.w3.org/WAI/WCAG21/Techniques/html/H84
Yes, I guess so. However, since we don't have PhP 7.4.1 in our developer environment yet, I cannot test this to see its behaviour. Is there an example of this anywhere? |
Here is the Javascript code that currently implements the jumpMenu functionality: vufind/themes/bootstrap3/js/common.js Lines 465 to 468 in 39d76b4
As you can see, it is causing a form submit on "change", which is definitely not appropriate. Honestly, I'm not sure why we never noticed this before, since as you're pointing out, it makes standard keyboard interactions annoying and weird, even if you are a sighted user. I think there are at least two possibilities for solutions here if we want to improve accessibility without adding buttons everywhere (and maybe each of these solutions is more appropriate in one context or another, so we might want to implement both for use in different places): Possibility 1: make the jumpMenu logic a little more sophisticated -- store a "data-initialValue" attribute in the select tag, and add Javascript logic which submits on "blur" instead of on "change" and only if the newly selected value differs from the data-initialValue. This could be combined with a screen reader text explaining that "changing this control will cause a page reload." Possibility 2: replace jumpMenu select boxes with different controls that are essentially pop-open menus of links (using the mechanism currently found in the language selector). This may not be technically possible in all scenarios, but it is certainly possible in some.
I don't mean to imply that I'm not taking the suggestions seriously -- I think this type of input is very valuable, and I'm certainly not trying to deny the existence of a problem. I'm just doing my "due diligence" to understand exactly what underlying problems led to the suggestions, and to make sure that we address the problems in the best way that we can. I've found that as a general rule, accessibility fixes improve the experience for all users, not just those using screen readers, because they often involve improving predictability of behavior and conforming more closely to standards and conventions. In this instance, adding many buttons to the interface just seemed like it might not be the best approach... and I'm pretty confident we can solve this problem with less disruption to the interface people have become accustomed to.
You don't need to be able to run #2378 to see the behavior I'm describing -- it's the functionality of the language drop-down menu at the top of the page. Since you did not change that here or report a problem with it, I strongly suspect that this control did not cause problems for your accessibility testers (since it is basically a link list that you can expand/contract, rather than an input element with side effects). The #2378 PR doesn't change the form of the control, it just refactors code to create a reusable template for building that kind of control. Once the work there is finished, we could apply some of the newly-created tools to also build a "jump menu" template, and then all of the controls with this type of functionality would use the same markup. It would make accessibility fixes like this less work to implement, and also offer an opportunity to easily override the behavior locally (for example, if one institution decided that it would be better to have explicit buttons on all drop-downs, and another institution felt otherwise). |
I see a Possibility 3 here: Remove jumpMenu from all select boxes so they behave as normal select boxes should. We could still add an explanatory text if we wanted to. The language dropdown, for example, is perfectly in order.
This would be a perfect way forward, I think |
I think your Possibility 3 may actually be the same as my Possibility 2, but we're just not using the same language to describe it. The language dropdown you mention is not an HTML select control, though it looks and acts like one. My possibility 2 suggests using this same technique to replace jumpMenu boxes. I suspect that you might mean the same thing by your possibility 3. Otherwise, if we just remove jumpMenu from an existing HTML select control, it will not function without a submit button (or some equivalent custom Javascript), and that sort of puts us back where we started. :-)
Great! Perhaps @crhallberg has some thoughts on next steps.... |
Thank you for your patience - especially for such a minor nitpick. I really like the button to activate the new change. I've been seeing that in a lot of places recently. The design seems good, I'm just wondering if the icon is the best choice. Maybe an arrow or something more like "go" or "submit" action would make sense. |
My turn to apologize - I didn't even make the conference this week, sorry! @demiankatz Thank you for your lengthy explanation. I see what you mean now. My intention behind removing the ´jumpMenu` class was ideed to bring the select elements back to their original function with a submit element to trigger the change. That's exactly what was listed as the flaw: That select elements cause page changes without being explicitly 'asked' for them. @crhallberg The icon was just a suggestion from the colleague who implemented it, so we're more than happy if it gets improved upon. |
I'm glad this thread has been revived, but I'm not entirely sure I know what our next step is. Do we want to refactor the controls to use a link list equivalent to the language control, or do we want to reintroduce the buttons everywhere? Or do we perhaps want to choose one solution or the other on a case-by-case basis depending on the context? One other consideration about the link approach is that it may increase the scope of things that web crawlers can find, though hopefully a In spite of possible crawler issues, I still like the idea of using links to avoid adding buttons to the already-cluttered search results screen (assuming that this is acceptably accessible, but if the language control passes, I would expect that it's okay). Things like the library card selector might not be adversely affected by simply using a button, but maybe @EreMaijala has a stronger opinion about that than I do (as a primary developer/user of that feature). Let me know how I can help, whether it be by refactoring, or helping to find a button we can all agree on, or something else. Hopefully we can prevent this from getting stalled again. :-) |
The basic idea behind the accessibility criticism was that a select element (sort results / results per page) should behave like a select element -- a dropdown (language menu) like a dropdown. So the way forward, imo., would be that we turn the select elements into select elements again, where feasible with triggering buttons. Unless we decide to turn some of them into dropdown lists. So the way forward could be either. As the main coders I would leave the choice to you. We are currently running happily having removed the jumpmenu feature on all select elements (such as sort and results per page) and added triggering buttons on all our catalogs. The change has been accepted without any issues or complaints.
Agreed, but beware of German Autumn holidays!! ;-) |
Thanks, @ckaz! I think it probably makes the most sense to try to finish #2378 first so that we have an easily reusable component to build drop-down link lists in places where we want them (unless we decide to just go with select-and-button everywhere). We decided at the Summit that we will begin using the Community Call to discuss outstanding work on 9.0, so I am going to assign this to the 9.0 milestone, and that will ensure that it gets discussed on next month's call if it has not been resolved sooner than that. With @crhallberg on vacation this week, we're going to have to pause once again for a few days. :-) |
Hi Demian, I could easily revise this for VuFind 9 @crhallberg wrote:
If you have any preferences as to the wording/icon use, pls. let me know and I'll incorpoare them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to merge dev into this PR to refresh the CI but I don't have permission.
Thanks for trying @crhallberg. However, I think this is on hold anyway until we can finish #2378. I think there are one or two things awaiting your input over there -- you might want to review the conversation history there for unresolved threads. |
With #2378 finished, we can now apply the MenuButton component to some or all of the jumpmenu instances highlighted here. Would @ckaz or @crhallberg prefer to take the first step on this? |
Replacing these items with menubuttons would go against the two-step behavior created by this pull request. Menubuttons (as currently defined) are filled with links that navigate elsewhere. This is definitely clearer from an accessibility standpoint than a JS controlled select element, but I want to make sure this is the right direction before I proceed. |
Hi there, I don't think it would be the right choice to replace a structural select element (such as the list items and sort by selectors in result list or the select items in advanced search) with a menu button. Others, like the language button or menu items, yes, but with a standard structural select element, I'd still rather expect a select element -- no? But maybe I'm missing something? Cheers! |
@ckaz, my main concern here is avoiding turning what is currently a single-step process into a two-step process. If we can avoid adding an extra click/button submission, users will be less annoyed, as long as they understand what is happening. Perhaps this is a situation where it is useful to look at other widely-used systems. For example, I notice that Amazon's sort feature is a |
I understand your concern from a sighted user's perspective. Amazon try to circumvent this by having a standard which they hide from the sighted user and adding a kind of dropdown-button construct in its place, which is similar to the new menu-button component ... when you work the replacement construct, it adds aria-pressed to the real , virtually operating it and vice versa -- when you operate the , it operates the fake select construct too I still doubt it is fully accessible/not confusing to non-sighted users by it may be a way forward. Here's both elements made visible (hope it shows, if not, the image is at https://user-images.githubusercontent.com/8236627/211004392-6c0902dd-6a8a-4c43-a5d4-a3822ae6a3dd.png) |
Thanks, @ckaz, maybe we can discuss this in a little more detail at tomorrow's Community Call. I don't have really strong opinions one way or the other, but I'd like to see if we can get some consensus around the best path forward! |
I'd prefer less effort for us ;-) So I'd rather do without the fake construct that Amazon has. We have the refresh buttons on top of the search results on several catalogs by now -- without any complaints. Do you know how often Sort By and Item Numbers per page are used? We could also approach this from a more pragmatic perspective; if they are used very rarely, a refresh button could be the best way forward. If used often, an alternative construct might be required. What do you think, @crhallberg ? |
Thanks, @ckaz, these suggestions all sound reasonable. I brought this issue up on the January community call and it did not prompt any obvious strong feelings. Another thought, which I think serves as a complement to your suggestions: as we've discussed, some of the situations where we currently use jumpMenu can be replaced logically by the MenuButton component. For those that do NOT make sense, maybe we should define a different named component. The trick is coming up with the name for it -- SearchParameterSelect, maybe? By applying this abstraction and moving the rendering logic to a sub-template, we then make it really easy to customize if different institutions have different opinions/priorities. We might even be able to offer submit-button vs. auto-submit as a theme.config.php setting (though I'm not advocating for proliferation of config settings if nobody cares about the difference). |
I agree that the refresh button is the way forward but if that's the case the menubutton component isn't the way to go. |
@crhallberg, I think we're in agreement that MenuButton is not appropriate for Sort/Limit in search results... but what about some of the other contexts touched by this pull request, like library card selection or search scheduling? I'm not really sure without delving deeper, but I think there's a possibility that some cases will lean one way and others a different way. |
Thanks, @EreMaijala, I think what you propose is the best compromise yet, since it has a functional justification that benefits users of all types in addition to aligning better with standard behavior and increasing accessibility (again, not that those latter two goals aren't good enough on their own, but it's a bigger win when we can get all three!). This also pokes an obvious hole in my earlier proposal of creating a SearchParameterSelect component, since it decouples the submit button from the selects... though maybe we could have a SearchParameterControls component which composes a set of SearchParameterSelect components, or something like that. I don't know if that's a good idea or overengineering at this point. :-) |
I don't think a Javascript component is appropriate in this case, since the button won't be changing any content on the page (quite the opposite: doing a page refresh). A wrapping form (which I think is in place) and submit button should do the trick. |
@crhallberg, I wasn't proposing a Javascript component necessarily -- just a template-based "component" to reduce redundancy in the code for reusable aspects of the templating. |
We've had multiple discussions about this internally, and I try to summarize here. While the proposed solution and my proposal would be fine for screen readers, they don't provide optimal user experience in other cases. We think that a JS dropdown with links (properly equipped with aria attributes) would be a better solution, similar to the language dropdown. Ultimately we could avoid the whole issue by updating results lists via AJAX, which would eliminate the need to reload the whole page, and would also allow us to maintain focus in the selected element. This would also allow use of e.g. select fields without go buttons etc. AJAX results would also provide additional benefits like reducing the amount of data transferred and server resources used for all the functionality around the results (facets, recommendations etc.). But since this is larger undertaking (which I'm interested in working on), the dropdown would be our favorite for the time being. |
Thanks, @EreMaijala, this sounds reasonable to me. What do you think, @ckaz? Maybe we can adopt the link-menu approach for 9.0 and then switch to AJAX result loading for 10.0 as an incremental path forward? |
@EreMaijala , @demiankatz sure, good idea and maybe the best of both worlds. I'll have to opt out of working on this for the time being as we've got a massive launch next week ... |
I think at this point, our best bet may be to simply close this PR and start a fresh one with the approach agreed upon here. @crhallberg, do you have time to start this, since @ckaz is currently occupied? I think it's just a matter of removing all the jumpMenu code and classes and introducing the menu-button component to the following places:
What does everyone think? |
Sounds perfect to me! Sorry, I have had so little time lately -- we're still fire-fighting issues. I'll be back on the VuFind front hopefully in a week's to 10 days' time! |
Thanks, @ckaz! If this is something of interest to @stephanieleary, this might be an area where she could help out while @crhallberg and I are wrapping up other issues. If you're interested, let me know and I can provide more details should you need them. This commit demonstrates the type of refactoring that needs to be done: 0ba425f |
Yes, I can help with this. New PR preferred? |
Thanks, @stephanieleary! I think in this case, a new PR will be easier, since we're taking a different approach from the work here. Once a new one is opened, I can close this one. |
…and subjects (vufind-org#2492) Co-authored-by: Ere Maijala <ere.maijala@helsinki.fi>
@stephanieleary, just checking in to see if you've had any time to work on this, or if there's anything @crhallberg or I can do to help! Happy to collaborate on work in progress if that would be helpful. |
I've opened #2814 as a new place to work on solving the underlying issues discussed here. I've included a TODO checklist with all outstanding work so we can keep track of progress. I think at this point it is safe to close this PR, so I am doing so -- let's continue the conversation on the new one as needed. |
This adds refresh buttons to the limit and sort select boxes (result list/favorites) to prevent unexpected page reloads.
It affects limit and sort select boxes in search results and favorites lists.
The background for this was reported to us during our accessibility checks: Select elements must not automatically change users' context when keyboard navigation moves between options. See also, e.g., https://fae.disability.illinois.edu/rulesets/FOCUS_4/
Generally, one may navigate select boxes by opening them (SPACE on most Windows settings, I believe) and making the selection. OR one may navigate straight away, selecting options with the arrow keys.
Due to the jumpMenu class, both menus reload the page immediately when using the arrow keys-method (there is no chance to choose) and place the focus to the top of the page! This may be disorienting to non-sighted (and other users).
The suggestion of the accessibility testing people was to add a refresh button next to the select boxes.
Desired behavior: Tab to the new sort box in result list and select an option with the arrow keys. Press TAB again and confirm on the new button with ENTER.
For comparison, tab to the sort box in the Live demo and use the arrow keys ...
There will be more focus loss-related issues I'd like to discuss in subsequent PRs.
TODO