Modal Accessibility Issues

[zackkrida]

Our modal component and our "arrow popup" component (usage 1) and usage 2 have major accessibility issues and should be extracted into their own components. They should later be moved into vue-vocabulary as well.

This is a react component, but the rendered HTML is an excellent reference for what features an accessible modal should support: https://reach.tech/dialog/

• [x] Extract Modal.vue component
• [ ] Fix a11y:
  • [ ] Add attributes aria-modal="true" role="dialog"
  • [ ] esc key closes the modal
  • [ ] Click on the modal backdrop (the darkened area behind the modal window) closes the modal
  • [ ] Trap focus within the modal

I have actually implemented all of this on a previous open-source vue project, which could be a god reference: https://github.com/sdras/cssgridgenerator/pull/9

[dos077]

Hmm, was going to implement this, but looking at the code, this is the touch/mobile portion of the search filter module. If it's moved to a different component, the search filters component will be separated into two files, will be confusing for future maintenance.

IMO there should be one div that's styled according to screen size and the accessibility fixes should apply to the unified element. The current breakpoints are defined with Bulma, I think I can just import the breakpoints in the scss and maintain consistency without resorting to hide/show toggle two otherwise identical divs.

[zackkrida]

@dos077 FYI a generic AppModal component has been created already: https://github.com/creativecommons/cccatalog-frontend/blob/develop/src/components/AppModal.vue

It has a default slot so any contents can be added inside.

Basically, that component needs the additional a11y controls mentioned above, and then <div class="modal"> here: https://github.com/creativecommons/cccatalog-frontend/blob/develop/src/components/Filters/SearchGridFilter.vue#L5-L25 would need to be replaced with <app-modal>. No need to create additional files for search filters.

Let me know if that makes sense and if not I can try to clarify further. Thanks for looking into this issue!

[dos077]

Reusable component is a good idea, but I still think hiding and showing two identical elements is not as efficient as just one element with responsive CSS. Anyway, since the component is already here, I agree it should be utilized then, just a bit of elbow grease.

[dos077]

Okay, ended up making a lot more changes than I planed... Going to list them here as well as the pull request so everyone should get a chance to see it:

• AppModal:

  1. remove close button, the close button is always inside the template content
  2. remove id prop, vue has its own dom management system, having an id that no one should call query on and may lead to duplicate warning is not a good idea
  3. added visible prop, controls whether the modal shows or not
  4. document listener when visible for keyup, so ESC will trigger $emit close
  5. changed the modal size to the original designs, otherwise it looks weird

• SearchGridFilter

  1. change top el from aside to div, the aside tag is not place next to main in browsePage, I think that makes more semantic sense.
  2. remove the duplicate conditional rendering, now the component just returns a FiltersList container.

• BrowsePage

  1. change the search column from div to aside
  2. added app-modal component
  3. use css class is-hidden-${breakpoint} to conditionally render between filters in modal or aside

A few more things I should point out about why so much changes:

1. Originally the filter modal (fix position) is nested inside a search column that has sticky position property. This is will not render correctly.
2. Bulma's applies { overflow-x: hidden; overflow-y: scroll } to most(all?)elements. This makes nesting sticky position element impossible, not that's easy to begin with.