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.

Sign up for GitHub

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

Jump to bottom

Add/uptake label providers in nimble-components #1328

Merged
merged 25 commits into from
Jul 13, 2023
Merged

Add/uptake label providers in nimble-components #1328

merged 25 commits into from
Jul 13, 2023

Conversation

msmithNI
Copy link
Contributor

@msmithNI msmithNI commented Jun 26, 2023 •
edited
Loading

Pull Request

🤨 Rationale

Partially addresses #1090 (Angular/Blazor changes will be in subsequent PRs)

👩‍💻 Implementation

See the HLD for the implementation plan for the label providers.

This PR creates the LabelProvider classes in nimble-components, and creates/uptakes label DesignTokens in nimble-banner, nimble-number-field, and nimble-table

🧪 Testing

  • Verified that each label appears in the expected spots in the DOM for the affected components. Note that the current labels are mostly not user-visible (except that grouped table columns will now have a 'Grouped' tooltip on that icon in the header). Future table changes (i.e. the column header menu) will introduce new visible label tokens.
  • Added autotests
    • For label providers: verify token naming, and that label provider attribute updates result in token updates
    • For controls using labels: verify that the label token values are used in the controls as expected
  • Updated Storybook
    • Each component using labels has a new "Localizable labels" row in their Storybook controls/API section
    • Under "Tokens" is a new "Label Providers" section. Each label provider has a section with a table that has the token name, JS property name, HTML attribute name, and English default value of the label.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.
    • Updated nimble-components Readme and Contributing docs with Localization sections.

@msmithNI msmithNI marked this pull request as ready for review June 26, 2023 22:22
@msmithNI msmithNI requested a review from rajsite June 29, 2023 00:03
@msmithNI msmithNI requested a review from jattasNI June 29, 2023 23:50
rajsite
rajsite reviewed Jun 30, 2023
View reviewed changes
packages/nimble-components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/nimble-components/CONTRIBUTING.md Show resolved Hide resolved
packages/nimble-components/README.md Show resolved Hide resolved
change/@ni-nimble-components-6eb6eef1-eec2-4f6d-a98f-f7182ecdad50.json Show resolved Hide resolved
packages/nimble-components/src/table/components/header/template.ts Show resolved Hide resolved
packages/nimble-components/src/label-provider/base/index.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/label-provider/core/label-tokens.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/label-provider/core/label-tokens.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/label-provider/table/index.ts Outdated Show resolved Hide resolved
@msmithNI msmithNI requested a review from rajsite July 10, 2023 20:58
@msmithNI msmithNI merged commit d5b19d5 into main Jul 13, 2023
@msmithNI msmithNI deleted the localizable-labels-1 branch July 13, 2023 23:23
@msmithNI msmithNI mentioned this pull request Jul 21, 2023
Merged
1 task
msmithNI added a commit that referenced this pull request Jul 25, 2023
@msmithNI
Remove banner dismissButtonLabel (#1376)
2c77e8e 
# Pull Request

## 🤨 Rationale

Follow-up tech debt from adding the label providers (#1090).

The banner's close button text can now be controlled by the
`popupDismiss` label on `nimble-label-provider-core`, so the
`dismissButtonLabel` property on `nimble-banner` is no longer needed. We
decided to remove the property in [PR discussion on
#1328](#1328 (comment)).

## 👩‍💻 Implementation

Remove property and update tests.

## 🧪 Testing

Updated autotests / Storybook.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@aagash-ni aagash-ni mentioned this pull request Aug 28, 2023
Merged
1 task
rajsite pushed a commit that referenced this pull request Sep 6, 2023
@aagash-ni @AagashRaaj
Rich Text Editor | Add label provider in nimble-rich-text-editor (#1459)
75dcf39 
# Pull Request

## 🤨 Rationale
Issue Link : #1288

Currently, `rich-text-editor` has `title` and `text content` for
formatting buttons (Bold, Italics, Bulleted List, Numbered List) and
this needs to be localized from the client side.

## 👩‍💻 Implementation

Reference PR for implementation: #1328

This PR  creates label DesignTokens in `nimble-rich-text-editor`

Note: Angular Integration will be taken up once after [this
](#1450 completion.

## 🧪 Testing

- Verified that each label appears in the expected spots in the DOM for
the affected components. Note that the current labels are mostly not
user-visible. Future format button additions will introduce new visible
label tokens.
- Added autotests
- For label providers: verify token naming, and that label provider
attribute updates result in token updates
- Updated
[Storybook](https://60e89457a987cf003efc0a5b-ohnshtynly.chromatic.com/)
- Rich Text Editor has a new "Localizable labels" row in their Storybook
controls/API section
- Under `Tokens/Label Providers` is a new `Rich Text Editor Label
Provider`.

## ✅ 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). -->

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

---------

Co-authored-by: Aagash Raaj <[email protected]>
@msmithNI msmithNI mentioned this pull request Sep 12, 2023
Merged
1 task
@m-akinc m-akinc mentioned this pull request Sep 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jattasNI jattasNI jattasNI approved these changes

@rajsite rajsite rajsite approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@msmithNI @rajsite @jattasNI