Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

[terra-clinical-item-view] Implement item view as unordered list #913

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

RayGunY
Copy link
Contributor

@RayGunY RayGunY commented Sep 6, 2023

Note: This PR is a little messy right now as it's pointing towards UXPLATFORM-9563. Once that's merged this will be able to be rebased with main and it'll clean up better.

Summary

What was changed:

  • Implementing the usage of unordered lists to contain the displays as list items
  • Split out the logic into more individual methods for readability/because there were so many different scenarios to handle
  • add guidance for consumers about adding a header for the item view

For two column layout (this includes the byRow formatting) the implementation of the unordered lists is slightly different. It works as an unordered list that contains the two columns as list items. Each column is an unordered list as well that contains the displays as list items.

For normal two column layout:
Unordered List
-> list item 1 (column 1 as UL) -> column 1's displays as list items
-> list item 2 (column 2 as UL) -> column 2's displays as list items

For by row two column layout:
Unordered List
-> list item 1 (row 1 as UL) -> row 1's two displays as list items
-> list item 2 (row 2 as UL) -> row 2's two displays as list items
-> list item 3 (row 3 as UL) -> row 3's two displays as list items
-> list item 4 (row 4 as UL) -> row 4's two displays as list items

When there is just a single display passed in to the item view we don't treat it as part of a list.

Why it was changed:
This was changes so we have greater context when using the screen reader of how this information is grouped.

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

Validation posted below, first two are how the implementation will read when the consumers adds their header. Also added a recording of the 'by row' readout since it is different than the other two.

VO-one-column-with-header.mov
VO-two-column-with-header.mov
VO-two-column-by-row.mov
JAWS-two-columns-by-row.mov
JAWs-one-column.mov
JAWS-two-column.mov

This PR resolves:

UXPLATFORM-9430


Thank you for contributing to Terra.
@cerner/terra

@RayGunY RayGunY self-assigned this Sep 7, 2023
@RayGunY RayGunY marked this pull request as ready for review September 7, 2023 16:21
Copy link
Contributor

@aalexanderj aalexanderj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one small nit

@RayGunY
Copy link
Contributor Author

RayGunY commented Sep 11, 2023

@eawww @sycombs @sdadn this is ready to be looked when you've got a moment

@RayGunY
Copy link
Contributor Author

RayGunY commented Sep 14, 2023

Posting in it's own comment - I was having trouble getting the header label to read out with JAWs. Dug around a bit and JAWs by default does not read out headers for lists. More on that here: https://stackoverflow.com/questions/36882441/jaws-not-reading-listview-column-headers

So since by default it doesn't read them out (presumably for some reason that the JAWs team has determined) this should be good in that regard. Just to be as thorough as possible I've been trying to figure out how to switch the settings so it will read out the header, but I've had no luck so far. I've tried to do these steps but I can't access the manager it seems? https://docs.oracle.com/applications/smartview/610/SVAAA/enabling_jaws_to_read_listview_headers.htm#SVAAA-smart_view_accessibility_book_4

I've posted recordings to at least show that it's reading them out as a list now. Does the above JAWs behavior sound reasonable? Not sure if I'll be able to get recordings with that setting working.

@RayGunY
Copy link
Contributor Author

RayGunY commented Sep 20, 2023

I've removed the header code, we've decided to have consumers add their own visible header and are going to advise in the documentation how/why to do that. I've added an accessibility section for the main site page which includes an example for single column and two column layouts. Also added a blurb about truncation, I saw the clinical item display page has one there too so I just copied it over.

I'll be adding similar but more detailed information on the actual accessibility guide, that will be done in this PR: #916

Screenshot 2023-09-20 at 10 05 55 AM

@vinaybhargavar @zxeleven @sycombs @sdadn @scottwilmarth this can be reviewed/re-reviewed when you've got some time

@RayGunY RayGunY changed the title [terra-clinical-item-view] Implement unordered list to create header/content relationship [terra-clinical-item-view] Implement item view as unordered list Sep 21, 2023
@RayGunY RayGunY force-pushed the UXPLATFORM-9563 branch 2 times, most recently from 4ebe431 to 456b749 Compare September 22, 2023 15:59
@sdadn
Copy link
Contributor

sdadn commented Sep 22, 2023

Does this depend on #912 ? If so, I'll review it once that gets merged to avoid messy diffs and merges.

Base automatically changed from UXPLATFORM-9563 to main September 25, 2023 17:11
@sdadn
Copy link
Contributor

sdadn commented Sep 25, 2023

#912 Has been merged, once you update it with main, I can review this.

@github-actions github-actions bot temporarily deployed to preview-pr-913 September 25, 2023 18:34 Destroyed
Copy link

@scottwilmarth scottwilmarth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still improvements that should be made, but there is not a clear path to manage the needed changes when considering passivity. These updates are not perfect, but they represent a better path and will be great to ensure better support for accessibility.

@sdadn sdadn merged commit d8c4b27 into main Sep 29, 2023
21 checks passed
@sdadn sdadn deleted the UXPLATFORM-9430 branch September 29, 2023 16:58
@sycombs sycombs mentioned this pull request Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants