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

Appointments header is showing on VBA services without content #17925

Closed laflannery closed 4 months ago

laflannery commented 6 months ago

Background

I added to test services to a VBA facility and in one instance the "Appointments" header showed when I'm not sure it should be because there was no content below it.

Description

Happy path that works properly I added a VBA service with the following options: - Service options: - Office visits: First-come, first-served - Virtual: First-come, first-served - Appointments: - Remove text - Uncheck Facility phone number - No additional phone numbers - No online scheduling On the FE, there was no appointments info at all, which was expected: ![Screenshot 2024-04-23 at 9 52 15 AM](https://github.com/department-of-veterans-affairs/va.gov-cms/assets/106678594/55f824a9-2b9a-41ce-aa60-e7a38c812968)

Bug

However, in the second scenario I did see FE info and I feel like I shouldn't have:

Is this heading conditional based on the Service option choices? And if so, would we be able to but some conditionals into Drupal to validate against this FE scenario? or at the very least maybe update the helptext - For example "If you have indicate that appointments are available or necessary please make sure to do something"? Or is the better option to conditionalize the header against something else?

Acceptance Criteria

jilladams commented 6 months ago

From refinement:

davidmpickett commented 6 months ago

Logic matrix is maintained in Figma: https://www.figma.com/file/eECF81nwktbnKX889HiKhl/VBA-Content-Specs?type=design&node-id=208-714&mode=design

Static copy for ease of references

Logic for displaying Appointment Info on Front End

However, this matrix was really meant to show scenarios in which all the Appointment fields should definitely be Hidden (because editor has indicated that appointments are not possible). The Show scenarios still should account for the possibility of no appointment data being entered and therefore the Header shouldn't show.

Ideally, in the scenario Laura has described that what we should actually be doing is enforcing Drupal input. If the Editor has indicated that Appointments are possible, there should be SOMETHING in the appointment section (a phone number, online scheduling, or some text). I have a ticket where I have been gathering some of the possible conditional validation situations: https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18031

I think this ticket can be refined as a FE ticket to be defensive against this scenario.

jilladams commented 6 months ago

(Just read this title as "Appointments header is showing on VBA services without consent" and had to double take for a sec.)

jilladams commented 5 months ago

From product sync:

Shouldn't block VBA staging review, signed off by Michelle / Michael.

laflannery commented 5 months ago

The issue is actually simpler than this and the matrix really should have nothing to do with the display of this header.

mmiddaugh commented 5 months ago

Example in Prod with scenario Laura describes above, from Coatesville VA Medical Center urgent care

Screenshot image.png
eselkin commented 4 months ago

I agree about the h4s all being the same level, because of the way that I constructed the new conditionals (variables that can be reused), we can conditionally push the appointments phone numbers down to h5 if we want. But I don't know what the view of using headers that high are.

davidmpickett commented 4 months ago

I agree about the h4s all being the same level, because of the way that I constructed the new conditionals (variables that can be reused), we can conditionally push the appointments phone numbers down to h5 if we want. But I don't know what the view of using headers that high are.

The long-term vision is to move away from headers for most of the interior of Service Accordions, so I would say let's definitely not add an additional tier of headers.

davidmpickett commented 4 months ago

I didn't have published content that met the specific criteria needed to test this on my Tugboat, so I just added some and am doing content release again. Should be here when ready: https://web-jdujzwgvp8kmj36hcfonqejlgtq5b6st.demo.cms.va.gov/birmingham-veteran-readiness-and-employment-office

davidmpickett commented 4 months ago

Verified on Tugboat Screenshot 2024-07-08 at 9 55 21 AM