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

[terra-table] Allow consumers to hide table column headers #1843

Merged
merged 21 commits into from
Oct 23, 2023

Conversation

cm9361
Copy link
Contributor

@cm9361 cm9361 commented Oct 18, 2023

Summary

What was changed:
The Terra Table component was updated to allow consumers to control the visibility of column headers in the table.

Why it was changed:
The change was made to maintain passivity with existing implementations that requires the column headers to be hidden such as the Stella Timeline.

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

N/A

This PR resolves:

UXPLATFORM-9638


Thank you for contributing to Terra.
@cerner/terra

@cm9361 cm9361 marked this pull request as ready for review October 19, 2023 14:01
@kenk2 kenk2 assigned kenk2 and cm9361 and unassigned kenk2 Oct 19, 2023
@chrismichalewicz
Copy link

@cm9361 Functionally it looks good, confirmed no headers are read in JAWS as well.

Copy link
Contributor

@eawww eawww left a comment

Choose a reason for hiding this comment

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

This wasn't clear in the jira but for the Stella Timeline case where the headers need to be visually hidden, they still need to exist to give that context to screen readers. A screen reader won't know to associate the external timeline as the column headers.

@eawww
Copy link
Contributor

eawww commented Oct 20, 2023

It gets the header info now! Thanks Charles. One last thing is the header cells shouldn't have buttons since they can't be accessed by anyone without a screen reader.

@cm9361
Copy link
Contributor Author

cm9361 commented Oct 21, 2023

It gets the header info now! Thanks Charles. One last thing is the header cells shouldn't have buttons since they can't be accessed by anyone without a screen reader.

Thank you for the feedback, @eawww. I need the logic from upcoming PRs to remove the button. I would not want to add that to this PR to prevent merge conflicts. What I can do is make the cell be 'not selectable'. This should give the desired behavior once the JIRAs in-flight are merged. Would that be acceptable?

@github-actions github-actions bot temporarily deployed to preview-pr-1843 October 22, 2023 01:35 Destroyed
@eawww
Copy link
Contributor

eawww commented Oct 23, 2023

@cm9361 could you point me to the jira or PR removal of the button is slated on?

@cm9361
Copy link
Contributor Author

cm9361 commented Oct 23, 2023

Copy link
Contributor

@eawww eawww left a comment

Choose a reason for hiding this comment

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

🥶 Cool beans! 🫘
Approving with the understanding that the removal of the button is being accomplished elsewhere.

@eawww eawww added ⭐ UX Reviewed UX Reviewed and approved. and removed UX Review Required labels Oct 23, 2023
@cm9361 cm9361 merged commit 22787bd into main Oct 23, 2023
21 checks passed
@cm9361 cm9361 deleted the remove-column-headers branch October 23, 2023 21:03
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