Update license explanation tooltip

[dhruvkb]

Fixes

Fixes #761 by @Ad-Mk
Fixes #423 by @obulat
Fixes #541 by @panchovm

Description

This PR

• adds icons and explanation for deprecated CC licenses
• uses VPopover to render license explanations
• refactors old components using defineComponent and 'V-' prefix
• replaces hardcoded data in store with calculated data

Testing Instructions

0. Check out the PR and run the dev server.
1. Search for any term and open the filter sidebar.
2. Check that the license explanation appears inside the bounds of the window.

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[obulat]

This is a great improvement!

There are a couple of things that don't match the mockups:

• there is not 'Cross' close button at the top right corner
• the heading font size should be 14px

[sarayourfriend]

Thanks for adding the deprecated license icons! That's a great addition 🎉

Navigating to an image result from the search page results in the app crashing. It does work if you reload the result page directly in SSR though. This appears to be an issue with media.license being undefined instead of a string.

In addition to what Olga pointed out, there's a couple other issues I noticed:

1. Keyboard navigation is mostly fine, but we should change the initial focus element to be the license explanation text rather than the Read more link
2. On mobile, opening the filters really messes with the layout of the filters on the page. It's also difficult to close the popover by tapping outside of it, which is probably more a Popover bug than it is an issue with this PR. I tested the popover component on mobile in Storybook and they worked fine, tapping outside of the popover closed it as expected. I'm wondering if there's some edge case with mobile Safari where tapping inside a fieldset doesn't register a regular click event or something? I can't reproduce it in Firefox for Android (neither the interaction issue nor the layout issue).

Here's a screenshot of the layout issue on mobile Safari. Note the license buttons are pushed all the way next to the license text.

[sarayourfriend]

aria-hidden is being applied on the entire license element explanation, not just the decorative icon.

The icon should have role="presentation" rather than aria-hidden, that's my bad for the incorrect recommendation.

Otherwise this is looking good.

[sarayourfriend]

For pdm and cc0 or any other "single element licenses/marks" this can be left out as it's redundant with the heading of the popover.

[dhruvkb]

@sarayourfriend please review this PR. If all's okay, I'll resolve the conflicts and merge it.

[sarayourfriend]

I don't currently have access to VoiceOver or NVDA so if someone else can test that it would be ideal. Based on the code changes the problems I was concerned about should be addressed so I'm removing my request for changes.

I'll work on setting up a Windows VM this week so I can continue testing NVDA and start testing JAWS as well.

Changes applied

[fcoveram]

I noticed two things:

1. I do not see the tooltip when pointer is over the question icon
2. The question icon should change to dark-charcoal in hover.

Here is the mockup for reference and a screenshot of expected implementation. The component is from the WordPress Design Library.

[sarayourfriend]

I do not see the tooltip when pointer is over the question icon

We don't currently have a component to handle tooltips. Can we implement this feature in a separate PR? This one is already really long running. The icon does have an aria-label so it's accessible for screen readers at least.

The main reason I'd like to push the tooltip into a separate issue/PR is that tooltips have their own set of complicated a11y considerations that would probably delay this PR even more than it's already been. Whomever implements the tooltip component will need to do some research into the best practices for them. I think it can be implemented using the VPopover or its composables but the a11y requirements are slightly different (especially around focus handling and disclosure interactions).

[fcoveram]

Absolutely, that makes sense. Putting my comment aside, excellent work @dhruvkb, as always ✨

[obulat]

We could add a tooltip like that by adding title attribute, but I'm not sure how accessible it is. Both the title and the button label seem to be read.

[sarayourfriend]

We could add a tooltip like that by adding title attribute, but I'm not sure how accessible it is. Both the title and the button label seem to be read.

IIRC SRs behave slightly differently when there's both title and aria-label, some read the title, some read the label, some do other funky things to I think.

[sarayourfriend]

@panchovm can you verify if this PR happens to fix the issues you filed in #759?

[zackkrida]

Here's a video of VoiceOver in Safari, using keyboard navigation. Please turn the sound on for the full experience.
It seems that:

• the close button is focused by default in the popover
• the popover contents are not read to the user

CleanShot.2022-02-22.at.11.22.00-converted.mp4

[sarayourfriend]

@zackkrida I think those are meant to be addressed in a separate issue as discussed in this conversation #850 (comment) (you need to expand the resolved conversation to see it).

[zackkrida]

LGTM. I've tried to read through the (extensive) history of updates since I approved this, and I believe everything is done. I created a new issue for the high-priority bug of screen readers not announcing the popover contents: #987

[zackkrida]

@panchovm if you have an opportunity today, could you test this pull request and then open it on your iPad? You should see a permalink like http://172.16.227.1:8443/ that will allow you to open the app running on your computer on your iPad.

We have a theory that this pull request fixes the issue you were seeing in #759

If @dhruvkb has merged this before you get to do this, you can test it with the main branch. Thank you!

[dhruvkb]

@panchovm I merged this pretty quickly because it was repeatedly developing conflicts. You can test this on your iPad by directly opening search-staging.

[fcoveram]

I tested it on my iPad on Firefox and Safari and the header bug (#759) has gone. It works perfectly. Excellent work ✨