perf(a11y): Make modals accessible

[sabicalija]

close #210

[sabicalija]

I've created a PR and added a first draft how I would approach modals. @klues Can you please have a look about the current changes? Opening/closing modals is possible, style needs to be updated, but I wanted to verify/discuss this approach before making changes in modal-related all files.

I've had a look in all modal components. All of them look like this, right?

File: src/vue-components/modals/addMultipleModal.vue

<div class="modal">
  <div class="modal-mask">
    <div class="modal-wrapper">
    <div class="modal-container" @keyup.27="$emit('close')" @keyup.ctrl.enter="save()"> ... </div>
  </div>
</div>

I think we can drop CSS classes 'modal', 'modal-mask', and 'modal-wrapper'.

[klues]

Thanks for your contribution! 👍

I've had a look in all modal components. All of them look like this, right?

correct.

I think we can drop CSS classes 'modal', 'modal-mask', and 'modal-wrapper'.

Yes, currently the CSS is in modal.css. Probably we can drop most of it and what's needed should go to modal.vue.

I like the approach, looks good to me! What we should consider to add to modal.vue:

• "X" to close the modal at the top right
• handlers for Esc for closing/aborting or Enter (or Ctrl+Enter) for saving - here it should be possible to pass a function to be called on save() from the actual modal to the base modal.
• default templates for the modal-footer like a OK/Cancel buttons or maybe even a possibility to define custom buttons by simply passing an array of labels and callback-functions?! I don't know to which extent this makes sense, if it should also be possible to configure things like the two rows of buttons from the "edit element modal" (see below) just by passing parameters to modal.vue?! Something like <modal :btns-footer="[{label: 'Cancel', row: 1, faIcon: 'times', fn: cancelFn}, ...]"/>

Edit Element Modal:

[klues]

For the headers, probably also some generic code makes sense - possibility to simply pass a header text for the default case.

Related to this: a generic modal for replacing the browser-internal confirm() modal would also make much sense - for a more consistent UX:

[sabicalija]

I think we can drop CSS classes 'modal', 'modal-mask', and 'modal-wrapper'.

Yes, currently the CSS is in modal.css. Probably we can drop most of it and what's needed should go to modal.vue.

Yes, I agree.

I like the approach, looks good to me! What we should consider to add to modal.vue:

• "X" to close the modal at the top right

Yes, we can add that. We could do it in two ways, depending on what we want to achive:

• Static 'X': this would be part of the template of modal.vue itself and not inside of a <slot> element. Thus, it won't be able to "overwrite" it. Nevertheless, we could provide the option (e.g. via properties) to display the "X" or not.
• Slot "X": this would be part of the "header" slot for instance, but could be overwritten by providing custom slots and would require a new implementation of "X" if it is still necessary.

• handlers for Esc for closing/aborting or Enter (or Ctrl+Enter) for saving - here it should be possible to pass a function to be called on save() from the actual modal to the base modal.

Do you mean to pass, for instance, two functions like handleEsc and handleEnter to customize functionality of modals? Currently, Esc triggers the closing of <dialog> elements without the need of an implementation to toggle its visibility.

• default templates for the modal-footer like a OK/Cancel buttons or maybe even a possibility to define custom buttons by simply passing an array of labels and callback-functions?! I don't know to which extent this makes sense, if it should also be possible to configure things like the two rows of buttons from the "edit element modal" (see below) just by passing parameters to modal.vue?! Something like <modal :btns-footer="[{label: 'Cancel', row: 1, faIcon: 'times', fn: cancelFn}, ...]"/>

I think it might make sense to implement Cancel button and Ok button, but if further buttons are required (I don't know how often that's the case, and if an (accessible) layout can be defined upfront), I'd suggest to go for custom slots. One would need to define the <button> elements and could register the handlers there directly. At least, if I understand it correctly. Like:

<template>
  <modal>
    ...
     <slot #footer>
       <button @click="handleCancel">Cancel</button>
       <button @click="handleOk">Ok</button>
       <button @click="handleCustomAction1">Custom Action 1</button>
       <button @click="handleCustomAction1">Custom Action 2</button>
     </slot>
  </modal>
</template>

Furthermore, we could provide modal-related CSS classes to allow easier/quicker design and less repetition, e.g.

<button class="modal-btn row-2 ???">Custom Action 1</button>

For standard cases, where only Cancel and OK are required, one could maybe pass handlers like that.

<template>
  <modal :modal-cnl="handleCancel" :modal-ok="handleOk">
    ...
</template>

I think this is a more understandable design when customizing functionalities of modals, rather than passing arrays around. Furthermore, it keeps the modal component cleaner and separates the responsibilities more appropriately. At least, from my current understanding.

Is there a difference between the handleEsc and handleSave discussion before or are handleCancel and handleOk alternative names describing the same thing?

Edit Element Modal:

[sabicalija]

For the headers, probably also some generic code makes sense - possibility to simply pass a header text for the default case.

Related to this: a generic modal for replacing the browser-internal confirm() modal would also make much sense - for a more consistent UX:

Yes, I think both suggestions are really good and make sense! I guess, the header text, like "Edit grid item" from the previous screenshot should be passed as a parameter to the modal. Right?

[klues]

Static 'X': this would be part of the template of modal.vue itself and not inside of a element.

I would opt for this. I think almost every modal should have the same possibility to close it. An exception maybe would be modals which require some user input, but I don't think we have something like this until now ("confirm" modals should evaluate "X" as "cancel"). But if we need it, I would simply hide the "X" via a property.

Do you mean to pass, for instance, two functions like handleEsc and handleEnter to customize functionality of modals? Currently, Esc triggers the closing of

elements without the need of an implementation to toggle its visibility.

OK, then handleEsc probably isn't needed, we don't have modals where some custom actions must be performed on canceling a modal. And handleEnter/handleSave, we would need to handle the default "save" button in the footer - which probably also should have a default keyboard handler like Enter or Ctrl + Enter.

I think it might make sense to implement Cancel button and Ok button, but if further buttons are required (I don't know how often that's the case, and if an (accessible) layout can be defined upfront), I'd suggest to go for custom slots. One would need to define the elements and could register the handlers there directly. At least, if I understand it correctly. Like:

Yes, currently there's only the edit element modal with 4 buttons, I think all other ones have two buttons. So for now, we can stick with your suggestion and adapt/extend it, if needed at some point in the future.

<modal :modal-cnl="handleCancel" :modal-ok="handleOk">

As said, handleCancel probably isn't needed, but maybe there should be an additional property :modal-ok-label in order to make it possible to override OK with e.g. Save. And then :modal-ok-handler instead of :modal-ok in order to make it clear.

I think this is a more understandable design when customizing functionalities of modals, rather than passing arrays around.

If we would have more modals with many buttons, I would like the idea to not have to think about the layout every time - but a said, if it's only the exception like now, I agree with you.

Is there a difference between the handleEsc and handleSave discussion before or are handleCancel and handleOk alternative names describing the same thing?

Same thing.

Yes, I think both suggestions are really good and make sense! I guess, the header text, like "Edit grid item" from the previous screenshot should be passed as a parameter to the modal. Right?

Correct. But in this case, it will be a custom header, since it also shows a small image and the label of the element you're currently editing. And it reminds me of another thing that would be good in the global modal.vue: some kind of "help" icon next to the "X" (see screenshot above) and a property like :help-location which defines which location is opened at clicking on the icon.

[sabicalija]

Another question came up: should the default modal implementation contain the "X" for closing and a button "Cancel" for canceling? If so, are they any different or do they behave the same? Do we need both?

[klues]

I think both should be part of the default modal implementation, but should result in the same action, both are simply closing the modal without additional actions. I think it makes sense to have both, since some users will search for the "X" at the top and some for the possibility to "cancel" at the buttons. Other implementations like bootstrap are doing it in the same way.

[sabicalija]

I'm wondering about $t. What is it? Where is it loaded? Where's the source?

<div class="modal-footer">
    <div class="button-container">
        <button @click="$emit('close')" :title="$t('keyboardEsc')">
            <i class="fas fa-times"/> <span>{{ $t('cancel') }}</span>
        </button>
        <button @click="save()" :title="$t('keyboardCtrlEnter')">
            <i class="fas fa-check"/> <span>{{ $t('insertWords') }}</span>
        </button>
    </div>
</div>

This snippet is from templateModal.vue. Is this just a template implementation or is it used somewhere?

I've seen that the Cancel and OK button, usually look the same, with almost identical implementation. However, in the templateModal.vue I don't understand why $t("insertWord") is used instead of $t("ok").

This is my current suggestion:

<template>
    <dialog ref="modal">
        <div class="modal-header">
            <span>{{ title }}</span>
            <slot name="header"></slot>
            <button @click="close"><i class="fas fa-times"></i></button>
        </div>
        <div class="modal-body"><slot></slot></div>
        <div class="modal-footer">
            <slot name="footer">
                <slot name="cancel-button">
                    <button @click="close" :title="$t('keyboardEsc')">
                        <i class="fas fa-times">{{ $t('cancel') }}</i>
                        Cancel
                    </button>
                </slot>
                <slot name="ok-button">
                    <button :title="$t('keyboardCtrlEnter')">
                        <i class="fas fa-check">{{ $t('ok') }}</i>
                        Ok
                    </button>
                </slot>
            </slot>
        </div>
    </dialog>
</template>

This way, one add custom content to the header, and we can make title and esc optional by parameters.

Since Cancel and OK are almost the same for all components, it might make sense to allow to make either the whole "footer" or "cancel" and "ok" buttons individually customizable via slots.

Cancel functionality is the same for all components, so it can be part of the modal.vue implementation. However, I don't know about the "ok" button. What does save() do (usually)? Does it make sense to make it part of modal.vue too?

[klues]

Questions answered personally ;)

[sabicalija]

Do you have an idea (a klue :nerd_face:) what of the previous changes might have caused this error message?

mainVue.js:117 TypeError: Cannot read properties of null (reading 'id')
    at Proxy.render (gridEditView.vue?./node_modules/vue-loader/lib/loaders/templateLoader.js??ruleSet%5B1%5D.rules%5B1%5D!./node_modules/vue-loader/lib/index.js??vue-loader-options:211:37)
    at Vue._render (vue.esm.js:2581:28)
    at VueComponent.updateComponent (vue.esm.js:3021:27)
    at Watcher.get (vue.esm.js:4205:33)
    at Watcher.run (vue.esm.js:4281:30)
    at flushSchedulerQueue (vue.esm.js:3267:17)
    at Array.eval (vue.esm.js:3902:20)
    at flushCallbacks (vue.esm.js:3824:18)

[klues]

To which lines oft code does the browser take you, if you click the link in the message in the console?

[sabicalija]

Initially I thought, the error may be caused by some changes in gridEditView.vue, but I've only changed some lines. And the error doesn't seem related.

This is the line. I didn't click on it initially but looked it up manually. But the code loaded in the browser uses different sources..

Still, it's not obvious for me. I'd have to have a more detailed look.

[sabicalija]

This is the entire console output. There is another line causing an error, that I didn't mention before. Maybe it helps..

[klues]

The error seems to happen in gridEditView.vue - so what does the browser show when clicking on this link in the console?

[klues]

this.component = component is the line in MainVue.js where a view component is loaded via router.js (e.g. gridEditView.vue).

[sabicalija]

To which lines oft code does the browser take you, if you click the link in the message in the console?

This is the relevant line.

Maybe it's caused by the fact that the parent/root component inside of the setNavigationModal.vue component changed from <div> to <modal>?

[klues]

I assume it's this line where the setNavigationModal is included: https://github.com/asterics/AsTeRICS-Grid/blob/master/src/vue-components/views/gridEditView.vue#L32

The property grid-id is taken from gridData.id and gridData seems to be not defined in your case. Normally v-if="showNavigateModal" should prevent this, since showNavigateModal only gets true after gridData is already loaded.