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

Add localized labels to remaining table sub-elements #1519

Merged
merged 18 commits into from
Sep 20, 2023

Conversation

mollykreis
Copy link
Contributor

Pull Request

🀨 Rationale

Add localized labels and titles to remaining elements within the nimble-table.

This is part of #1151

πŸ‘©β€πŸ’» Implementation

Add new label tokens for required strings and use them within the table.

πŸ§ͺ Testing

  • Manually tested that the labels show up correctly in the accessibility tree*
  • Manually tested that titles are appropriately shown on elements in storybook
  • Created a branch that re-introduced initial rows in the Angular example app table and verified that lighthouse accessibility check now passes.

*As part of this manual check, I found that the labels on our group header icons don't work as expected due to #1510

βœ… Checklist

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

@mollykreis mollykreis requested a review from msmithNI September 12, 2023 17:49
@mollykreis
Copy link
Contributor Author

@msmithNI, could you buddy this PR for me?

Copy link
Contributor

@msmithNI msmithNI left a comment

Choose a reason for hiding this comment

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

(Reminder that once this is checked in, the Angular + Blazor label providers need corresponding changes)

@mollykreis
Copy link
Contributor Author

@atmgrifter00, do you mind doing a second buddy review on these changes? Specifically, can you provide a review on the changes to the way that we are optionally adding an additional "column" for the row selection checkbox and group all button?

@mollykreis mollykreis marked this pull request as ready for review September 19, 2023 16:29
@mollykreis mollykreis merged commit 60947ef into main Sep 20, 2023
@mollykreis mollykreis deleted the label-table-elements branch September 20, 2023 16:12
mollykreis added a commit that referenced this pull request Sep 22, 2023
# Pull Request

## 🀨 Rationale

The new labels added in #1519 need to be exposed in Blazor

## πŸ‘©β€πŸ’» Implementation

Followed existing pattern to expose and test new labels

## πŸ§ͺ Testing

Ran unit tests

## βœ… Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.
mollykreis added a commit that referenced this pull request Sep 22, 2023
# Pull Request

## 🀨 Rationale

The new labels added in #1519 need to be exposed in Angular

## πŸ‘©β€πŸ’» Implementation

Followed existing pattern to expose and test new labels

## πŸ§ͺ Testing

Ran unit tests

## βœ… Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.
rajsite added a commit that referenced this pull request Sep 25, 2023
…ch other (#1555)

# Pull Request

## 🀨 Rationale

With #1519, we can re-introduce initial rows to the table in the Angular
example app without failing the Lighthouse accessibility check. I
decided that this was also a good chance to make the tables in the
Blazor and Angular example apps consistent with each other.

Resolves #1195 
Resolves #978 

## πŸ‘©β€πŸ’» Implementation

Updated the Blazor and Angular example tables to:
- Use consistent models to represent the table data
- As part of this, I made the Blazor example app have deterministic data
and removed the use of `Faker`
- Have the same set of columns and column configuration
- Load initially with 10 rows
- Add 10 more rows to the table when the _Add rows_ button is pressed

## πŸ§ͺ Testing

- Ran both example apps and make sure the tables behave identically and
have the same data
- Verified that the Lighthouse accessibility check passes (part of the
pipeline)

## βœ… Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

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

---------

Co-authored-by: Milan Raj <[email protected]>
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.

5 participants