ADR: Implement an accessible navigation system #43
Conversation
matt-bernhardt
force-pushed
the
post-107-adr
branch
4 times, most recently
from
80e4a53 to
9093316
Compare
JPrevost
left a comment
JPrevost
left a comment
There was a problem hiding this comment.
I had a few comments. I'll need a bit more time to absorb the content before I can come back and approve or provide feedback on what is blocking my approval. My initial reaction is that I agree with the decision, but I need to sit with that a bit more before formally approving. Thanks for the clear, detailed write up of the problems space and options.
|
|
||
| * The most significant drawback is that it imposes some operating costs on users | ||
| of assistive technology that are not ideal. Our recent accessibility testing | ||
| [included some details about this problem](https://github.mit.edu/Accessibility-FY2024/omeka/issues/4#issuecomment-182190) while we discussed this menu issue. |
There was a problem hiding this comment.
I don't have access to this link. Please either move the content to a location we all have access to (maybe our own Jira ticket that tracks that GitHub issue if we have one) or summarize the important aspects here.
There was a problem hiding this comment.
I'm re-working this section with a summary and direct quotation from Rich - but would like to still like to the issue queue for transparency's sake, even if not everyone has access to the link. Would you be okay with that?
There was a problem hiding this comment.
Not really. I think either the core team should have access to supporting links or we should move the content to some place we can access it (like in our own tracking Jira ticket, etc). Linking to something nobody but you can access feels like the opposite of transparency :)
There was a problem hiding this comment.
I agree that links should go someplace we all have access to - so it makes sense to me that we get you and Adam access to the review issues (which goes along with what we talked about yesterday, that we should just generally have access to repos like that, rather than only project team members).
Because we're having this discussion about a library we're deciding not to implement, I don't know that there will be a Jira ticket that could get this information - the implementation ticket will be focused more on describing the current work, rather than work we decided not to do.
If the re-wording I did this afternoon doesn't go far enough in your eyes, I'm happy to expand the quoted material here, or put together a summary in Confluence of the problem space. I'll also get you and Adam access to the DAS repo.
| * The markup requirements for this library do not immediately align with the | ||
| markup generated by Omeka. While this is manageable by either maintaining a | ||
| set of custom navigation templates (which other themes also do) or by writing | ||
| some javascript to manipulate the DOM prior to instantiating the menu, it does |
There was a problem hiding this comment.
FWIW, I'd likely lean towards maintaining custom nav templates over scripts to change the DOM before nav kicks in. Both require custom code that could break if Omeka changes upstream expectations, but the custom templates feel like they would be easier to maintain (I could be wrong, but in general fixing this as close to the sources as possible feels better to me and that's what a nav template feels like)
There was a problem hiding this comment.
For Accessible Menu, I would agree with you - the amount of customization to the markup needed for the menu to fire was best handled via custom templates rather than an intermediate javascript layer.
For the current SmartMenus system, though, that modification is extremely minimal - adding sm and sm-mint to the overall ul element. For that level of customization, I'd rather have a single line of javascript than to take on two different navigation templates (one for the top nav menu, and another called recursively for any submenu).
For the bespoke option we're talking about adopting, I don't think we'll be forced to make this sort of choice - but I'm happy to discuss this more.
There was a problem hiding this comment.
Yeah it's complicated at best. I guess my gut feeling is we'll eventually end up forking the nav templates into our theme so the sooner we accept that the less complicated we make this. I understand fully the desire to not fork the nav though as this is exactly the type of think that leads to annoyances when upgrading administrate in ETD.
|
|
||
| ## Decision | ||
|
|
||
| We will develop, and maintain, a small set of javascript and stylesheet rules |
There was a problem hiding this comment.
Can you explicitly name the option we are choosing. i.e. "We will implement `Option 4: A bespoke solution. We will develop..."
There was a problem hiding this comment.
I think I'm in the same boat as Jeremy, in that I need to sit with this for a couple of days. Here are my main reservations:
- It feels like we'd be reinventing the wheel. As you note, someone must have already built a viable accessible menu library. (But, as you also note, none seem to exist, at least as open source code.)
- Given how rarely we've maintained the style guide, it feels risky to add another frontend library to the list.
- When I think about the UI designer/developer position that may come up soon in UXWS, this feels like the sort of task that would be well suited to that role. (Not that we couldn't do it, but getting in the business of building JS libraries reduces our capacity elsewhere, and that seems like part of what that role is supposed to help with.)
I think I'm leaning toward approval, though, for the following reasons:
- As previously noted, it seems like a viable library is not currently available and will not be in the near future.
- It's entirely possible that we could build this now and hand it off to the UI designer/developer, assuming we end up hiring for that role. So, it could end up being some upfront work that we don't have to maintain in perpetuity.
- The approach presented feels minimal enough that it doesn't seem onerous to develop or maintain. Even if it does remain in our domain, it shouldn't bee too bad if we keep up with it.
I think my main remaining question has to do with the severity of the issue. I.e., is it on the level of "this isn't good, but we can accept it for now," or is it more on the level of, "this must be fixed asap"? The phrase "challenging, if not impossible" to refer to screen reader navigation suggests the latter (as does the presence of this ADR), but I'm curious if there's an option to wait a bit to see if the SmartMenus developer releases a prod-ready v2.
| features](https://configurator.smartmenus.org/). | ||
|
|
||
| However, at the time of writing [SmartMenus 2 is only available via one alpha | ||
| release](https://github.com/vadikom/smartmenus/releases) - and the maintainer has [cautioned us against using it in a production |
There was a problem hiding this comment.
If the frequency of past version tags are any indication, we could be waiting a long time for anything beyond alpha...
| ourselves or by a dependency maintainer). | ||
|
|
||
| * Cons: [These types of navigation systems are likely not going to be fully | ||
| accessible](https://moderncss.dev/css-only-accessible-dropdown-navigation-menu/). There are elements of the WCAG standards which cannot be satisfied |
There was a problem hiding this comment.
This feels like a dealbreaker (which is unfortunate, because a CSS-only menu is strangely compelling to me).
There was a problem hiding this comment.
Agreed, on both points.
|
|
||
| #### Drawbacks of a bespoke approach | ||
|
|
||
| The maxim "if you aren't using a framework, you are building one" applies here. |
There was a problem hiding this comment.
This is a really important point. Thinking about the style guide, I have some concern that a new frontend framework we build may end up languish in our maintenance backlog after a few years.
matt-bernhardt
force-pushed
the
post-107-adr
branch
4 times, most recently
from
c7f5a2e to
afffd6d
Compare
matt-bernhardt
force-pushed
the
post-107-adr
branch
from
afffd6d to
872c18e
Compare
|
In retrospect, I probably should have added some context to my approval. 🙂 I think this most recent commit accurately summarizes where we've landed on this as a team. Thanks for writing it up, and for taking the time to experiment with the bespoke solution. |
The ADR text was written with various supplementary URLs in mind, which I'm now going through and adding while I do a bit of wordsmithing. There are still some sections of the document which need to be written.
With the realization that we would not be able to complete the bespoke navigation option in the time available, I have re-written the ADR to explain our decision-making.
matt-bernhardt
force-pushed
the
post-107-adr
branch
from
c55e671 to
6262273
Compare
Why these changes are being introduced
Recent reviews of the theme's usability and accessibility have caused us to try and search for a replacement for our current navigation library (SmartMenus).
How this addresses that need
Because of the complexity of this change, and the level of effort needed to support the new navigation system, it makes sense for all of us in EngX to consider what we are committing to in this navigation system, to make sure that we're comfortable providing support for the chosen system.
Side effects of this change
This PR is the ADR only, identifying the path we will follow to replace the menu system. The actual change to the theme will happen in a subsequent PR.
Relevant ticket(s)