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
98 stars 69 forks source link

Front End: Full VBA regional subsection content #14957

Closed xiongjaneg closed 8 months ago

xiongjaneg commented 1 year ago

Description or Additional Context

Some services accordions are at a regional level. See Slack thread

Resources

Acceptance Criteria

Team

Please check the team(s) that will do this work.

xiongjaneg commented 1 year ago

@xiongjaneg create follow up ticket similar to Facility ticket. Edit: Done.

swirtSJW commented 1 year ago

To connect the facility query to get the service region, you need to include a reverse entity ref lookup on this field image

swirtSJW commented 1 year ago

Here is the relationship diagram for all the connections to the VBA Facility content type. https://prod.cms.va.gov/admin/reports/content-model/entity-diagram/node/vba_facility?max_depth=2

Any connection that is an arrow pointing to the red dot, is a reverse entity reference. Any pointing away from the rest dot is an entity reference. The name on the connectors are the fields that make the conntection.

swirtSJW commented 1 year ago

Actually I need to follow up with @davidmpickett because I think my pointer to the vba_region_facility_list might be incorrect. I won't know for sure until I understand where this data lives on the comps.
Slack discussion

xiongjaneg commented 1 year ago

Screenshot from 16th minute

Screenshot 2023-09-28 at 10 27 55 AM
swirtSJW commented 1 year ago

Followup from 16th minute.

xiongjaneg commented 1 year ago

Blocked in sprint 94 per Slack thread

xiongjaneg commented 12 months ago

Still blocked

jilladams commented 8 months ago

From 1:1 today, @maxx1128 noted he's gotten Jordan's review via a screencap video (Slack thread), plans to tag @laflannery for review in a Tugboat, and is feeling this may be ready to PR/demo next week.

@maxx1128 pls update if I misstated anything there.

maxx1128 commented 8 months ago

@jilladams, this looks accurate to me 👍 My upcoming tugboat instance will also have the service region accordions updated to V3.

maxx1128 commented 8 months ago

Update for end of Sprint 102: this ticket is blocked due to some expected merge conflicts with @eselkin's work on facility and regional services. Once that is set, the accessibility review is complete, and any other needed changes are made, this should be able to be completed in the next sprint.

laflannery commented 8 months ago

Headings:

Phone numbers:

laflannery commented 8 months ago

@maxx1128 I have finished my accessibility review, I know you already started looking at/updating based on my comments. I added a note about the telephone numbers to my original comment. I also found the original conversation about the "Can't find the service you are looking for" heading. We don't have an answer quite yet but I did sync with Michelle and this is on her radar. I would hold on this for now until we get direction from her.

laflannery commented 8 months ago

@maxx1128 Final decision on that second header comment is that we have decided to leave "Can't find the service you are looking for" as an h2 - it is a sibling of the other service headings and we should treat it as such. If that means is has to be in the "On this page" component, that's ok

jilladams commented 8 months ago

https://dsva.slack.com/archives/C0FQSS30V/p1706639135108009

tests pass after making banner testing not dependent on the whole page rendering with old data that doesn't match with the way the new data renders.

A11y feedback implemented and available in Tugboat (fyi @laflannery ) for verification, which will happen over S103 boundary.

laflannery commented 8 months ago

@maxx1128 I just reviewed the tugboat again and the headings look good

Some comments on Phone numbers:

maxx1128 commented 8 months ago

@laflannery Looking at these points again this morning:

  1. aria-describedby is dynamic. If there's a custom label set in the CMS for a number, that's what's used in the screen reader text.
  2. Whenever an editor adds an sms and tty number, they're used along with any other numbers with the same properties and styling. Is there anything specific or different that needs to be done with these types of numbers? In the CMS they have the same list of properties.
  3. I double-checked and saw that I missed adding aria-describedby for when it's the facility's phone number. I will add that in now for the next tugboat release.
laflannery commented 8 months ago

@maxx1128 Answers below:

  1. & 3. Excellent and Thank you!
  2. These properties in the DS component trigger certain things to happen so that they will be displayed/read differently.
    • These differences are:
      • TTY actually puts TTY in front of the number so folks know that this is a TTY number and not just a regular phone number
      • SMS swaps the tel: in the href with sms: so that folks know that this is also not a phone number but rather it should be used as an SMS number
    • If I'm understanding correctly, you are saying that regardless of which TYPE of phone number ad editor selects in Drupal, there is currently nothing unique coming from the query so that we will not be able to differentiate these and therefore set SMS on type SMS and TTY on type TTY? image
maxx1128 commented 8 months ago

@laflannery That's correct, as things are right now, the SMS and TTY numbers render the same code as the other types. I'll look into getting these changes made for these specific types of numbers.

laflannery commented 8 months ago

@maxx1128 We might want to hold on that and ask @xiongjaneg because it might be out of scope and not MVP for this particular ticket

maxx1128 commented 8 months ago

Changing the component based on these types of numbers is just an extra property. I'll try updating the tugboat, it may be an easy win.

Screen Shot 2024-01-31 at 1 19 46 PM
maxx1128 commented 8 months ago

@laflannery looks like it worked! The extra properties appear on sms and tty numbers but not the others, and they update the markup as expected for each.

Screen Shot 2024-01-31 at 3 11 12 PM
laflannery commented 8 months ago

@maxx1128 Regional subsections look swell! :)

jilladams commented 8 months ago

Closing. This has been in prod for end of Sprint 103, verified in staging envs, and will require prod content for prod verification. We can cut new tickets for any danglers.