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

Angular integration for label providers #1360

Merged
merged 41 commits into from
Jul 24, 2023
Merged

Angular integration for label providers #1360

merged 41 commits into from
Jul 24, 2023

Conversation

msmithNI
Copy link
Contributor

@msmithNI msmithNI commented Jul 18, 2023

Pull Request

🀨 Rationale

Partially addresses #1090 (Blazor changes will be a subsequent PR)

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

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

This PR adds in nimble-angular:

  • NimbleLabelProviderCoreDirective and NimbleLabelProviderTableDirective (standard wrappers)
  • NimbleLabelProviderCoreWithDefaultsDirective and NimbleLabelProviderCoreWithDefaultsDirective: Apps will use these directives to automatically initialize the label providers with the localized strings (which use $localize from @angular/localize, which is now an optional peer dependency for nimble-angular)
  • 2 new secondary entry points, @ni/nimble-angular/label-provider/core and @ni/nimble-angular/label-provider/table
    • In my prototype, until I added separate entry points, as soon as an app pulled in new nimble-angular version, Nimble's localized strings would be pulled into their resource files (even without depending on the new modules). Whereas, with secondary entry points, the app needs to explicitly import NimbleLabelProviderCoreModule and/or NimbleLabelProviderTableModule to get the strings pulled in, which seemed like better behavior.

πŸ§ͺ Testing

  • Added autotests for new directives
  • Update Angular example app to pull in the label providers the same way apps will, i.e. <nimble-label-provider-core withDefaults></nimble-label-provider-core>. Note that there's not additional translation logic in the example app though, so this just uses the English strings (so basically a no-op, just there as an example).

βœ… Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.
    • Updated Readme and contributing docs for nimble-angular with Localization info

package-lock.json Outdated Show resolved Hide resolved
@msmithNI msmithNI marked this pull request as ready for review July 18, 2023 22:02
Copy link
Contributor

@jattasNI jattasNI left a comment

Choose a reason for hiding this comment

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

Changes look good to me but my vote is to wait for the deps update to go in and then clean up the package changes in this PR. So I'll wait to approve until after taking another look at the package files when that's done.

@msmithNI msmithNI enabled auto-merge (squash) July 24, 2023 18:39
@msmithNI msmithNI merged commit 238805c into main Jul 24, 2023
7 checks passed
@msmithNI msmithNI deleted the localizable-labels-2 branch July 24, 2023 18:46
rajsite pushed a commit that referenced this pull request Sep 6, 2023
…ich-text-editor (#1477)

# Pull Request

## 🀨 Rationale
Issue Link : #1288

Angular Integration for the rich text component label providers to get
localized from the client app when using the rich text component ( For
now only `rich-text-editor` has labels ).

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

Reference PR for implementation: #1360

This PR adds `NimbleLabelProviderRichTextDirective` in nimble-angular.

## πŸ§ͺ Testing

- Added autotests for new directives

## βœ… 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 <aagash.maridas@solitontech.com>
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.

None yet

4 participants