Create the Content Report flow

[dhruvkb]

Fixes

Fixes #543 by @panchovm
Follows up hotfix PR #719 addressing issue #713
Completes a pending TODO in #682 by @krysal

Description

This PR updates the content report flow to match the new mockups using the new components.

Testing Instructions

In the Storybook:

0. Check out the PR and run the Storybook.
1. Navigate to the VContentReportPopover story.
2. Play with it.

On the actual frontend:

0. Check out the PR and run the dev server.
1. Navigate to the single image result view.
2. Find the 'Report this content' button.
3. Play with it.

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.

[zackkrida]

The flow is looking excellent, wanted to flag some early z-index issues:

[fcoveram]

I noticed the following.

Focus styles should be pink.

The Related images section covers the popover.

The idea of the character counter was to limit the message length by showing the remaining characters until reaching zero. However, I do not know what is a reasonable character limit for a message like this.

I forgot the mention and design that placeholder text should be dark-charcoal-70 to meet AA level. My mistake

I am seeing the close button slightly bigger. It should be the close-small element.

I also noticed other minor details on the "report content" action, but I did not add them as not sure whether those belong to the component or not.

[dhruvkb]

@panchovm

• the pink color for the focus will be addressed by Update color 'primary' to 'pink' after redesign #800
• z-index issues are fixed in 8232f3d
• the text field has a minimum requirement of 20 characters (without which the report cannot be submitted) and max length of 500 (which is virtually unreachable). So I felt that showing the entered characters was more useful than counting down from 500?
• increased placeholder contrast in 107c76f
• fixed the size of the close icon in d5dd924

[dhruvkb]

Cleaned up the Git history.

[sarayourfriend]

the text field has a minimum requirement of 20 characters (without which the report cannot be submitted) and max length of 500 (which is virtually unreachable). So I felt that showing the entered characters was more useful than counting down from 500?

This seems reasonable to me. I also think a character count is less confusing than a count-down from the limit in my experience. If we wanted to display the limit we could do something like `${characterCount} / 500`.

[sarayourfriend]

This is SUCH a welcome clean up of this component. It's amazing how much easier it is to use the form and the code is looking really clean and easy to follow now. Massive kudos to that!

However, I think we need some input from @panchovm on this component's usability, in particular whether a popover is really the best solution for this problem.

There are three issues I see with the popover being used for this:

1. When selecting one of the two options that increase the height of the popover (DMCA and Other), the popover becomes too tall to fit on a reasonably sized screen. You can see this problem present on my own massive 16 inch Macbook Pro screen (though admittedly I have my UI scaled up, to help my eyes, I think this will effect 13 inch and smaller screen users which are more common than the larger screen sizes).

Gravacao.de.Tela.2022-02-04.as.06.54.28.mov

This could be helped if we made the popover content wider, but could still cause problems for smaller screen sizes, especially because the popper positioning will default to rendering above the disclosure in many cases, but then be covered by the header.

2. We should consider whether this is a good use case for the popover component. A couple of reasons for this come to mind:
   2a. We are setting hideOnClickOutside to false. To me this probably communicates a sense that the form is currently "too small" and easy to accidentally click outside of it and ruin your progress in filling it out.
   2b. The form, when open, especially on a small scree, dominates the screen and because of 2a and the current behavior of the "Cancel" button, is slightly difficult/unintuitive to quickly close it if opened by mistake or curiosity.
   2c. We are running into various positioning issues already, even on a full-sized laptop screen.

These problems make me wonder whether we should use a dialog-style modal for this form instead. It would largely appear the same as the popover, but have its own independent scrolling, and receive focus from the page by intentionally hiding the rest of the page behind it and a modal backdrop. The current modal component would need some modifications as it's currently optimized for the single-result page design and not a more condensed "true dialog" type of interaction that this form would require... namely tighter horizontal margins.

3. We need to fix the z-index of the Popover content relative to the header. Popovers should appear above everything, always, period, so we might as well throw a custom z-index class on it for something absurdly high like z-9999 and call it a day. If we keep this form inside a popover, this change will block this PR. Otherwise we can open a separate issue for it.

I think I would like to propose keeping this in a popover for now, but opening an issue for @panchovm to rethink this as a modal or some other experience that would be more intuitive, easier to use, and not have the current positioning and size problems that are present with the popover.

However, this PR does introduce some significant quality of life improvements for this form and does not cause new issues (aside from the z-index issue which must be addressed for popovers one way or another). We should also widen the form to make it more easily fit onto average sized screens.

I do also want to reiterate that this PR is really good work and unequivocally improves the popover version of the form in many important respects. 🚀

[sarayourfriend]

What is the goal for removing this? It makes the focus ring really "tight" around the text and the icon 🙁

[dhruvkb]

With the horizontal padding the report button appears noticeably out-of-alignment with the right side of the page.

[sarayourfriend]

I see.

@panchovm how would you like these cases handled? Perhaps we shouldn't use a focus outline in these cases? We could translate the button horizontally to align it with the end rule of the page but that could cause unwanted horizontal scrolling in narrower screens if we're not careful.

[fcoveram]

I would follow the Gutenberg approach, prioritizing the visual alignment for elements having no border like the Edit action in the gif below.

[dhruvkb]

That's how it currently is. The focus outline appears a little closer to the text, but brings the button closer to the right edge.

[sarayourfriend]

Okay, if it's fine for the focus-outline to hug the text like that then disregard this comment 🙂

[sarayourfriend]

focusable="false" and role="img" seem kind of contradictory to me. Shouldn't we use role="presentation" if it's something screen readers can safely ignore?

[dhruvkb]

Tbh I copied this bit from VCheckbox. Since the preceding input is fairly accessible to screenreaders, should we go one step further and aria-hidden this SVG entirely?

[obulat]

It would be nice to set aria-hidden to both the radiomark and the checkbox checkmark.

[fcoveram]

So I felt that showing the entered characters was more useful than counting down from 500?

Absolutely. That is way better. I thought we set a minor character limit, and showing a counting-down element as Twitter does-ish sounded good to me.

Now I think it's not very useful and I would eliminate it, the disabled button changes to be enabled once reaching the 20 characters and that style change looks sufficient.

[fcoveram]

To @sarayourfriend's points.

1. Height change

You are right about the height change concern on screen sizes. It feels clumsy when jumping between options. The reason behind this solution was having a single step for reporting. The previous feels odd when seeing a description step after selecting an option. But now, I am thinking of a different solution that keeps the description part (text-area component) optional for some choices and visible all the time. I can work on this soon.

2. Modal over modal on desktop

When designing this, I thought the same, and my conclusion was that having a modal over another modal can become tricky in the long run. My approach was having popovers on desktop and modals on mobile, but you are right about the attention we are putting into this flow compared to other info pieces. I want to dive deeper into this and come up with an idea to solve this and other content interactions.

But supporting your message by saying that this work is an improvement of the existing one, and we should keep it until finding another solution where we all agree upon.

[sarayourfriend]

Regarding the width: It's not a blocking suggestion. It also needs to be done careful with consideration for narrow mobile screens. The current version fixes worse problems from the original, so I think the width we can consider during the next design iteration of this rather than stalling this PR.

There's a couple accessibility issues I've found as well, but I think these are present in the existing popover so we can open a seperate issue to address them. But first, some blocking accessibility issues that introduce regressions from the existing form:

1. We should order the submit button first in the taborder. It should also have the focusableWhenDisabled prop set to true.
2. The DMCA form information is not obvious to a screen reader user. There's no way to know that new text has appeared on the page and nothing tells you that there's something you need to do. This will require some redesign thinking. Potentially we can put the existing text into the radio label as sr-only text? I tried this but I couldn't really get it to work. It also needs to be present for the screen reader when that particular choice is focused not just selected, as it is possible to focus without selecting an option using a screen reader. This would be a good-to-have for this PR, but I would time-box trying to solve the problem. If it proves to be too difficult to figure out right now, let's open another issue to address it soon.

Non-blocking but needs to be addressed ASAP:

1. The initial focus element is the first option in the pop over. This feels wrong to me because it skips over the description of the form and will leave users who are not relying on visual context with a lot less important information and context about the options they're choosing from. To fix this we'll need to port the solution from the modal in Allow focusing specific element when opening modal #695 into the popover. They both use the same composables and general approach to code organization so it shouldn't be too difficult, but is something that could be tackled in a separate issue and PR.

@panchovm Instead of showing the new text box and DMCA notice inline inside the radio group, I think it would do to show each conditionally after the radio group, between the reason choices and the close/submit button. This way when someone chooses a particular reason, the next item on the page that will be focusable is the explanation or text box for them to fill out.

I have to say I think this pattern is very difficult from an accessibility perspective. It's not impossible to use but without visual context there's a lot that will be missing.

From a keyboard accessibility perspective for sighted users I think it's working well already. But screen reader users who cannot rely on visual context will probably find this difficult and confusing.

When it comes to the ARIA properties of the checkbox, I think you're doing fine. We're using native input elements so there's not too much to be concerned about. There are some difficulties I found using VoiceOver where it wouldn't read the label of the first option when it is intially focused when the popover is opened, but this would be fixed by switching the initial focus element of the popover to be the first paragraph of text in the popover rather than letting it use the default of the first tabbable element in the popover.

[zackkrida]

I'm noticing a quirk where when the popover is open there are double scrollbars on the page. This has to do with the way we're handling the page layout with grid and can be fixed by adding the relative class to the outermost div in src/layouts/default.vue

[fcoveram]

I was testing @sarayourfriend's idea and it looks good as a temporary solution for the focus navigation order and the height change. I set a fixed height no matter the option selected and added an optional field for the mature content reason. It looks like this.

Option 1	Option 2	Option 3

---

This idea aligns with the exploration I started for the modal/non-modal version I would like to drop in the following weeks and discuss the best long-term implementation.

[dhruvkb]

@zackkrida I tried a few times (in different browsers) but I'm not seeing two scrollbars, can you share the steps to reproduce?

[sarayourfriend]

@dhruvkb I see them when the popover hangs beyond the bottom edge of the screen:

This is probably a bug with the popover component rather than this usage of it though.

[sarayourfriend]

@dhruvkb can we mvoe the close button to be before the popover content in the DOM order? Currently "shift + tab" from the radio options focuses the disclosure rather than the close button. Not a blocker though.

[sarayourfriend]

The fixed height design looks good @panchovm and the implemenation seems to work really well at first glance @dhruvkb. I'm logging off for the day but I will review this with some screen readers tomorrow morning first thing to verify that it is good to go. My guess is that this is going to be an even bigger improvement over the existing implementation than it already was going to be. Really nice work!

[zackkrida]

This looks phenomenal. Noticed another tiny bug—the button is aria-disabled in this state, but not disabled, and is still clickable. It would be good to both fix this button but also make the form submission code fail internally if there's not a valid description.

[sarayourfriend]

This looks phenomenal. Noticed another tiny bug—the button is aria-disabled in this state, but not disabled, and is still clickable. It would be good to both fix this button but also make the form submission code fail internally if there's not a valid description.

I believe this is intentional after setting :focusable-when-disabled="true" on the button. Otherwise screen reader users will not be able to see that there is a submit button for the form, it's just disabled until they complete the required fields.

Isn't there also some way to use form @submit to handle the submit button event that will fail on the user-agent level if a required field or any validation attributes on form inputs aren't met?

[zackkrida]

@sarayourfriend yes! The required and minlength properties on the textarea should prevent form submission.

[obulat]

I tested this in Firefox on mobile, and the report form looks and works really great, no problems with z-indexes or not fitting into the screen.
I just wanted to note that the reporting copyright infringement is very user-non-friendly on mobile: the Google form is opened as a top window, closing the search result, and I have to enter the URL of the previous page (which is difficult to find on mobile!). Why do we use Google forms for reporting?

[obulat]

A massive improvement over what we have now, and nice code style details such as frozen object as statuses.

[obulat]

It would be nice to set aria-hidden to both the radiomark and the checkbox checkmark.

[sarayourfriend]

I just wanted to note that the reporting copyright infringement is very user-non-friendly on mobile: the Google form is opened as a top window, closing the search result, and I have to enter the URL of the previous page (which is difficult to find on mobile!). Why do we use Google forms for reporting?

Oof, that's not a good user experience on mobile!

I suspect it's just easier than building the form ourselves and also easier for whomever is taking action on the reports... though Django Admin really makes that part easy for us to remedy as long as we build the front-end for the reporting form.

In the mean time, while the Google form is still with us, we could use the strategy in this article to pre-fill the search form:

https://www.labnol.org/prefilled-google-forms-200601

Definitely a separate ticket for that one though.

[sarayourfriend]

From an accessibility perspective this is working really well now. Kudos to @dhruvkb and @panchovm for adapting the requirements of the form and the design of it to be accessible!

Just one last, non-blocking suggestion from my perspective about the placeholder and minlength. If folks disagree with that suggestion then it's fine to merge. I'm also fine if we just create another issue to refine the placeholder text.