department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
99 stars 69 forks source link

Recurring Events "Ends" dropdown exists on the page at -1,000px #19162

Open srancour opened 2 months ago

srancour commented 2 months ago

Statuses

[2024-09-12] [Fran] Blocked until decision is made whether PW team keeps Events; then we can either transfer or prioritize.

Describe the defect

When an event is made to be recurring there is a dropdown for "Ends" after the "Repeat every" "Repeat" type dropdown in the DOM, but on the page it sits at -1,000px due to a left: -1,000px; on the .form-item--field-datetime-range-timezone-0-repeat-endcss class. This makes it so anyone on a screen smaller than 2915px can't see it, but it still exists and can be tabbed to with keyboard and a screen reader user still encounters it.

To Reproduce

Steps to reproduce the behavior:

  1. Edit an event that's recurring like Music Therapy Jam Session
  2. Tab though options after "Repeat every" input and dropdown
  3. Either see focus onto "Ends" dropdown if screen size is at least 2915px wide, or see focus move onto nothing and then back on second tab.

AC / Expected behavior

If the idea was to hide the dropdown to only allow for a single option, this should have been done through something like display: none; so that a keyboard and screen reader users couldn't still access the dropdown.

Screenshots

Screenshot of part of the dropdown visible to the left of the screen with a green focus on it Screenshot of the dropdown opened and on the 'After' option showing that the option to input text goes in under the 'On these days' option

Additional context

This seems to have been here for 2 years since the Recurring Events feature was completed at pull #7981. It looks like it came from a request to 'Remove "Never/After/On date" dropdown' in a comment on #7037 by @kevwalsh from 2021.

Desktop (please complete the following information if relevant, or delete)

jilladams commented 2 months ago

@laflannery despite not knowing the destiny of Events and whether we'll update them and when / where: Steve found an a11y issue, if you would like to put a defect level on it.

laflannery commented 2 months ago

@EWashb and @jilladams This is a regression I believe - This was resolved 2 years ago (almost to the day weirdly enough)

My best guess is that it might have had to do with the recent upgrade?

srancour commented 2 months ago

@laflannery if you look at the class in the code, it hasn’t changed since June of 2024 when the #7981 push happened. I also found this on prod on Friday before the update to Drupal went to prod, so I don’t think it had to do with this Drupal update. From what I could find in the linking tickets, there was that similar issue where the “All Day” checkbox was also at -1,000px, but I couldn’t find this as a bug previously found or fixed.

jilladams commented 2 months ago

I got a little lost in the link sauce there, but stepping through it:

I'm not finding a comment anywhere that we were able to verify the fix on #10131, so it was prob lost in the shuffle of no Steve response.

If / when we pick this up, we will want to think through how we might be able to modify the JS to remove the All Day field as well (rather than display: none / having it present in the dom) as well as the "Ends" field, based on that PR / comments.