mayflower icon indicating copy to clipboard operation
mayflower copied to clipboard

DP-28505 Replace the custom responsive table scrollbar with native one

Open ygannett opened this issue 2 years ago • 9 comments

Any PRs being created needs a changelog.txt file before being merged into dev. See: Change Log Instructions

Description

Table wrapper

  • Replace the existing scroll bar and button control with the native scroll bar overflow-x: auto;
  • Set up the responsive table wrapper focusable for keyboard navigation with arrow keys
    • The wrapper is focusable only when it has overflow content to enable scroll control by keyboard
  • Add a label to the responsive table container syncing with the table caption via aria-labelledby
    • Add a separate instruction for screen reader users #sr-instructions via aria-labelledby

Table caption

  • Add caption to the template to indicate the scroll bar to users
  • Style caption as a table title
  • Hide the scroll bar indication message when the table wrapper has no scroll bar.
  • When no table title with no overflow content, caption is hidden, not an empty element.

Related Issue / Ticket

Steps to Test

Screenshots

Use something like licecap to capture gifs to demonstrate behaviors.

Additional Notes:

Anything else to add?

Impacted Areas in Application

@TODO

Today I learned...

ygannett avatar Dec 12 '23 15:12 ygannett

Backstop failure is not related to the changes in this PR: https://output.circle-artifacts.com/output/job/d7a0c3b6-2431-4488-8431-e0ef11e85e94/artifacts/0/backstop/reports/html/index.html

ygannett avatar Jan 11 '24 20:01 ygannett

@ygannett , I'm still seeing the legacy responsive table in Mayflower Patternlab, can you please update them to use the up-to-date responsive table styles? Please let me know if I'm missing something.

The patterns I checked: https://mayflower.digital.mass.gov/patternlab/b/patternlab/DP-28505_scrollbar-aria/?p=viewall-atoms-table https://mayflower.digital.mass.gov/patternlab/b/patternlab/DP-28505_scrollbar-aria/?p=pages-information-details-responsive-table https://mayflower.digital.mass.gov/patternlab/b/patternlab/DP-28505_scrollbar-aria/?p=pages-information-details-toc-precontent-responsive-tables

clairesunstudio avatar Jan 22 '24 21:01 clairesunstudio

@clairesunstudio I wonder that's cache. I followed your links and saw the new scroll bar. Here is one sample. Screenshot 2024-01-24 at 9 31 03 AM

ygannett avatar Jan 24 '24 14:01 ygannett

@ygannett, I see the scrollbar but on mobile the table still inverts: Screen Shot 2024-01-24 at 11 40 16 AM

For the table patterns https://mayflower.digital.mass.gov/patternlab/b/patternlab/DP-28505_scrollbar-aria/?p=viewall-atoms-table, I still see the old versions. can you update those? I think we can clean up all the old JS and assets if they are not being used on mass.gov

clairesunstudio avatar Jan 24 '24 16:01 clairesunstudio

Ideally, the description and scrolling instruction should not scroll with the table. But since they are in the caption, they are currently inside of the table. And whether we determine to scroll or not it's based on the table width and the table wrapper width.

I think we should probably check the tbody width and/or the thead width instead of the whole table. So that:

  1. It will prevent the description and instruction from scrolling
  2. In case the description is long, and the table itself actually don't have overflow, I think currently it will trigger the overflow instruction

clairesunstudio avatar Mar 11 '24 18:03 clairesunstudio

Please resolve the conflicts on this PR and the openmass PR. Thank you!

clairesunstudio avatar Mar 11 '24 18:03 clairesunstudio

I see your point. The reason I put the scrollbar info in caption is that it'd be the most reasonable place as the content flow for the info for screen reader users since they know they are in the table and haven't reached the table content although that is not the best presentation for sighted users. Another thing to consider is which is better in the limited space, especially in mobile: 1. see the table title and the scroll info. all the time by cutting off the space to show more table rows at once, 2. show the table content as much as possible by hiding the table title and the scroll info. It depends what we put more priority than the other, I guess.

I played around with caption, table, and tbody. caption always stays within the table/tbody or thead width. The third screenshot shows that the longer caption than the table content doesn't go over the table content (tbody and thead) width. Instead, it gets wrapped within the table content width. The current implementation wraps the longer caption content than the table width to fit the table width.

Table width: 100% Caption width: 30% Screenshot 2024-03-12 at 2 46 46 PM

Table width: 100% Caption width: 100% Screenshot 2024-03-12 at 2 47 19 PM

Table width: 80% Caption width: -- Screenshot 2024-03-12 at 2 48 53 PM

ygannett avatar Mar 12 '24 19:03 ygannett

The backstop failures are not related to this PR's change.

ygannett avatar Mar 12 '24 20:03 ygannett

Thanks for the clear demo! So the caption length doesn't determine the table width, only the table data does.

To your point here:

Another thing to consider is which is better in the limited space, especially in mobile: 1. see the table title and the scroll info. all the time by cutting off the space to show more table rows at once, 2. show the table content as much as possible by hiding the table title and the scroll info. It depends what we put more priority than the other, I guess.

I just want to clarify that I meant horizontal scrolling not vertical scrolling, just so we are on the same page. I was thinking we could treat the title and scroll info in a normal content flow outside the table so that they don't scroll horizontally with the table. Just wanted to point out that I have not seen table title and description purposely layout like that. But if you think it's the right place for that info and the most accessible content flow, I'm ok with that.

clairesunstudio avatar Mar 13 '24 19:03 clairesunstudio