Feature/CSL-2007: group1 enchanced tooltips

[klown]

Place the content as given in Kristen's google documents and convert the popup tool-tips into popover dialogs.

JIRA: https://castudl.atlassian.net/browse/CSL-2007

[klown]

@mbrambilla,

After moving the content from Kristen's google docs into the tooltips template, there are no popup tooltips left. All are popover dialogs now.

In terms of having two templates, one for tooltips, and another for popover dialogs, I suggest moving the modifications made in popover_tip.html template file to a new file named popover_dialog.html, and restoring popover_tip.html to what it was. I'll do that next.

Kristen's documents are listed in the first two comments in the JIRA:

• menu, read-aloud, switch, and book actions: https://castudl.atlassian.net/browse/CSL-2007?focusedCommentId=36455
• student activity, settings, word bank, and context menu: https://castudl.atlassian.net/browse/CSL-2007?focusedCommentId=36537

[mbrambilla]

@klown Already working down a similar path for splitting the templates up, so far:

• renamed previous (pre-PR) popover_tip.html to tooltip_tip.html to better reflect model and functionality
• minor adjustments in frontend.js in showTooltip() to change references from popover to tooltip.
• created popover_tour.html using your updated version
• prototyped using a single object list to create popovers - end goal it to use a list of objects to loop through and create multiple popovers
• currently revising tour.js and new template
• still need to work on chaining functionality

[klown]

@mbrambilla Since you have already split the templates and are working on the styles and behaviours, I will work on changing the model to reflect the kind of tooltip (popover vs. tooltip) and adjust the code accordingly.

Regarding the chaining functionality, that is not needed for this round for the so-called "group 1 enhanced tooltips", since there is no tour with these tooltips. But, if it's convenient for you to work on now, all the better in the long run.

[klown]

@mbrambilla ,

I have merged your changes with mine, and after some some minor modifications, the single popover-dialog tours work for the most part. There are some issues listed below. I will continue to investigate.

Also, I made a change to the TipType model, adding a kind field (tooltip vs. dialog.) But after merging in your changes, that field was superfluous. I have since removed it, since it is not needed at present.

There are three current issues: the popover arrow, the read-aloud button in the popover title bar, and the wordbank and context popovers.

Popover arrow:

• no arrow for the settings popover. The <div class="popover-arrow"></div> is present, but it has display: none.
• the arrow shows correctly for the readaloud, switch, views, and bookactions popovers.
• it does not display for activities and context but it is coded to not display there (line 194 of the popover_tour.html template).
• unsure of wordbank, context (see below).

Read-aloud button in popover titlebar:

• does not trigger read-aloud, but can be used to stop/pause reading if read-aloud has been triggered elsewhere.
• popovers affected: settings, switch, readaloud.
• works as expected for views, activites, and book_actions popovers.
• unsure of wordbank and context (see below).

No shows:

• The wordbank and context popovers do not show automatically. I managed to force the display of the context popover using the browser console. It had the correct look, but its read-aloud button did not work, showing the same behaviour as read-aloud on the settings popover.
• I'm unsure what's needed to make the wordbank and context popovers appear, but will look into that next week.

[klown]

@mbrambilla it actually is "SSETTINGS". I don't know why. Logging in as "theresateacher', on the Q/A site:

• https://clusive.qa.cast.org/res_reader/SSETTINGS shows the resource.
• https://clusive.qa.cast.org/res_reader/SETTINGS shows "Server Error".

[mbrambilla]

Could be a typo, but it is consistent with the resources.json

[klown]

It was one of the issues I ran into last week -- not finding the resource -- and fixed with this commit:
e231b52

[mbrambilla]

Should resolve most of the issues, but does introduce a new one.

New issue is that when using TTS to read the 'Read Aloud' popover, the positioning get messed up since the sidebar TTS play button becomes invisible. Looking into it now.

wordbank shows for me on the 4th load of a reading (once the tips_tipshistory table is set to have NULL for the last_attempt column. Note I set all the entries in the last_show, last_action, last_attempt to NULL for the user_id account I am testing with. Probably overkill, but it worked for me.

The usability of these popovers is very problematic on short screen due to the height of the popovers, so positioning can be off, have some strange repositioning when scrolling, or have the arrow be in the wrong place. Have not tested on mobile devices yet.

Also, screen reader a11y has taken a hit since the popovers are no longer within an aria-live container when shown, hence the use of a forced focus to the .popover element when shown.

[klown]

I can confirm that you fixed the jump in position when clicking the read-aloud button on the "Read Aloud" popover. 🎉

FWIW, the way I get the popover to show up is to temporarily change the first of TipHistory.ready_to_show() (in the tips models.py) to if self.type.name == <name of popover to test>: return True, and then subbing in 'settings', 'wordbank', etc.

Regarding the issues I cited above:

• the arrows display where they are supposed to,
• the read-aloud controls in the popover title bar works, and
• all of the popovers display.

[klown]

@mbrambilla, regarding:

Also, screen reader a11y has taken a hit since the popovers are no longer within an aria-live container when shown, hence the use of a forced focus to the .popover element when shown.

The APG has some advice for where to put the focus initially when a dialog is popped open. I looked at the first NOTE in their keyboard interaction section. It lists a number of places where focus could go including:

• "the first interactive element in the dialog",
• "a static element at the top of the dialog" when the dialog is larger than the viewport,
• etc.

A possibility for our popovers is to put focus on the main tip text, but that would require tabbing backwards to get to the video link. My current feeling is to put it on the popover heading. That would be the span with class="h3". Fleshing this out a bit, modify that heading markup and focus it when the page is loaded:

1. <span class="h3" role="heading" aria-level="3" tabindex="-1"> ... </span>,
2. document.querySelector("#tourContainer span.h3").focus()

I've tried that and it seems an intuitive interaction as a sighted user (sample size of one ;-) ), but I have yet to test with a screen reader. I'll do that next.

What do you think?

[klown]

I went ahead and committed the changes that I noted above regarding where focus is when the popover is shown initially. I tested it on Safari/VoiceOver and it seemed okay. But it's easily changed if there is a better way.

[mbrambilla]

The focus on the .popover element is consistent with Figuration's default Popover behavior. This is a generalized behavior instead of using the first focusable element because there could be instances when there is not a focusable item - never made sense to me why focus is recommended to be moved into potentially the middle (or end) of dialog content.

Additionally, if a .popover-header is found within the .popover an aria-labelledby attribute is automatically added the the .popover container linked to the .popover-header. Essentially your change should technically be handled already for a screen reader. Also by focusing on the .popover container, it is more likely the entire contents of the popover will be announced by a screen reader instead of just the heading text.

This reading of the entire popover is similar to what should occur with the tooltip version where they are placed within an aria-live region, the entire tooltip contents would be automatically announced.

The added focus statement is also needed because the Popovers are in manual trigger mode due to the complexity of chaining interaction. The show/hide methods are called functionally, not directly from a user interaction, since delays need to happen Also in manual trigger mode the focus does not get automatically moved to the popover on show, or restored back to the trigger button on hide.

[klown]

Thanks for the explanation. I will revert the focusing markup and code.

I made the assumption that screen readers would speak something along the lines of "Dialog, Word bank" when the dialog was shown, followed by speaking what is focused within that dialog. If the intended UX is that the entire dialog is spoken on open, then putting focus on the dialog container makes sense.

Regarding the aria-labelledby: because it is pointing at the div for the popover header, the accessible name is computed as, "Word Bank Read aloud controls" (using the word bank popover as the example). Since aria-labelledby is recursive, it gathers the text within the header div, including the label for the read-aloud region. But, the dialog's label is just "Word Bank", given by the heading span within the popover header. The aria-labelledby should reference just that span. It could be marked with a .popover-label, maybe ? I'm not familiar with Figuration details.

[klown]

@bgoldowsky Since Kristin's comment in CSL-2040 about the order for the tour implies that some of the group 1 popovers appear for multiple targets, this PR may require changes.

[bgoldowsky]

@klown ok - I'm thinking at this point it makes sense to wait for all of the tooltip stuff to be ready and merge to development in one shot rather than in the two separate groups.

[klown]

Thanks @bgoldowsky. In that case, I will close this PR and continue the work in the #328 PR

[klown]

Going forward, all of the work will be tracked in PR #328. Closing this PR.