Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create nimble-table-row and nimble-table-cell and use them in the table #921

Merged
merged 15 commits into from
Dec 16, 2022

Conversation

mollykreis
Copy link
Contributor

@mollykreis mollykreis commented Dec 14, 2022

Pull Request

🤨 Rationale

This PR creates the nimble-table-row and nimble-table-cell elements, which will only be used within the nimble-table, not as stand-alone elements exported from nimble-components. The property types on these elements will likely change moving forward, but creating the elements is a step closer to the correct structure of the DOM and separation of logic between the table, rows, and cells.

This PR also adds ARIA roles for a table.

This is part of #840.

👩‍💻 Implementation

  • Create nimble-table-row
    • Has ARIA role of row
    • Responsible for taking in the set of columns and row's record to create the cells. Currently, the set of columns is specified as an array of strings, but eventually this will be an array of column definitions
  • Create nimble-table-cell
    • Has ARIA role of cell
    • Responsible for rendering one cell of data. Currently this is based on a single value given to the cell, but eventually this will be based on a view template and cell data.
  • Add ARIA roles to the table & column headers

I think we will also create an element to represent a column header, but that was not in the scope of this PR.

🧪 Testing

  • Updated the page object for the table & verified unit tests continue to pass
  • Ensured there are no accessibility warnings/errors in storybook with the table
  • Ensured there are no visual changes introduced with this PR (styling will come as a separate pass)
  • Did not create specific unit tests for the nimble-table-row and the nimble-table-cell because we determined in an offline conversation that they did not seem necessary at this point in time. Until the need arises, we will plan to test table functionality with unit tests on the table itself, not its sub-elements.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@mollykreis mollykreis merged commit 1700ba3 into main Dec 16, 2022
@mollykreis mollykreis deleted the table-row-and-cell branch December 16, 2022 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants