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

feat(stepper, stepper-item): add numberingSystem property #5467

Merged
merged 5 commits into from
Oct 17, 2022

Conversation

benelan
Copy link
Member

@benelan benelan commented Oct 12, 2022

Related Issue: #4893

Summary

Adds numberingSystem property to calcite-stepper. A couple lines in calcite-stepper-item will need to be changed due to a refactor in #5466. The small change can go in which ever PR is merged last.

@benelan benelan requested a review from a team as a code owner October 12, 2022 23:29
@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Oct 12, 2022
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Aside from a considering another way to update the formatter options outside of render, this LGTM!

🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺
🎺🏇🎺🎺🎺🏇🎺🏇🎺🎺🎺🏇🎺🏇🏇🏇🎺🎺🏇🏇🏇🏇🎺🏇🏇🏇🏇🎺🏇🎺
🎺🏇🏇🎺🏇🏇🎺🏇🎺🎺🎺🏇🎺🏇🎺🎺🏇🎺🏇🎺🎺🎺🎺🏇🎺🎺🎺🎺🏇🎺
🎺🏇🎺🏇🎺🏇🎺🏇🎺🎺🎺🏇🎺🏇🏇🏇🎺🎺🏇🎺🏇🏇🎺🏇🏇🏇🎺🎺🏇🎺
🎺🏇🎺🎺🎺🏇🎺🏇🎺🎺🎺🏇🎺🏇🎺🏇🎺🎺🏇🎺🎺🏇🎺🏇🎺🎺🎺🎺🎺🎺
🎺🏇🎺🎺🎺🏇🎺🎺🏇🏇🏇🎺🎺🏇🎺🎺🏇🎺🏇🏇🏇🏇🎺🏇🏇🏇🏇🎺🏇🎺
🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺🎺

render(): VNode {
numberStringFormatter.setOptions({
Copy link
Member

Choose a reason for hiding this comment

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

Is there another hook we can use to update the options? You could watch for effectiveLocale, but that wouldn't handle changes to the parent's numbering system. Would that be a common scenario, you think?

Copy link
Member Author

@benelan benelan Oct 13, 2022

Choose a reason for hiding this comment

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

Yeah I'm not really sure there is a use case for dynamically changing locale and numberingSystem besides testing/debugging. I've just been updating the options to be safe lol.

I guess we could create an internal event or a mutation observer for the numberingSystem prop on the parent and a watcher for effectiveLocale, if you think that's better

Copy link
Member

Choose a reason for hiding this comment

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

I'm leaning towards only updating the formatter on effectiveLocale changes for now.

@benelan benelan added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 17, 2022
@benelan benelan merged commit 9ca3117 into master Oct 17, 2022
@benelan benelan deleted the benelan/4893-stepper-numbering-system branch October 17, 2022 19:24
anveshmekala added a commit that referenced this pull request Oct 19, 2022
* feat(rating): add built-in translations

* set asset dir

* add initial message load

* add effectiveLocale watcher

* fix outdated reference

* update lang switch helper to account for strings used in rendering

* feat(time-picker, input-time-picker): add numberingSystem property (#5301)

* feat(time-picker, input-time-picker): add numberingSystem property for localization

* wip: need to correctly apply numberingSystem on InputInput

* use NumberStringFormat to delocalize time

* cleanup

* delocalize time on blur

* test(input-time-picker): fix e2e failures and add numberingSystem story

* use formatter's format method instead of localize for performance

* cleanup

* fix(input-time-picker): don't pass through lang and numberingSystem to calcite-input

* fix test

* use NumberStringFormat's  setter instead of setOptions

Co-authored-by: Ben Elan <benelan@users.noreply.github.com>

* refactor(block): move hardcoded classes to CSS object (#5474)

* fix(input, input-number, input-text): fix input icons not displaying properly in Firefox (#5475)

#5417

* ci(next): clean docs build before deployment (#5476)

* ci(prepReleaseCommit): skip git sanity checks for next releases

* ci(next): clean docs build before deployment

* Revert "ci(prepReleaseCommit): skip git sanity checks for next releases"

This reverts commit 7cc2d8b.

Co-authored-by: Ben Elan <benelan@users.noreply.github.com>

* refactor(input-number, input-text): drop resize-icon selectors (#5477)

* fix(block): improve content layout (#5473)

#5422

* docs: update component READMEs (#5479)

Co-authored-by: jcfranco <jcfranco@users.noreply.github.com>

* docs: add backticks to props and values for consistency (#5485)

* docs: add backticks to props and values for consistency

* update prop ref

* review edits

* fix(date-picker): no longer hides year for zh-CN locale (#5344)

* fix(date-picker): no longer hides year for zn-CH locale

* add screener story

* fix locale typo

* fix  suffix placement

* docs(changelog): add missing breaking changes (#5469)

* feat(stepper, stepper-item): add numberingSystem property (#5467)

* feat(stepper, stepper-item): add numberingSystem property

* use setter for NumberStringFormat's options instead of setOptions

* change formatter options in effectiveLocale watcher

* cleanup

Co-authored-by: Ben Elan <benelan@users.noreply.github.com>

* fix(flow-item): fix scrollContentTo (#5487)

#5414

* ci(next): fix next releases take 3 (#5497)

* ci(next): fix next releases take 3

* cleanup

Co-authored-by: Ben Elan <benelan@users.noreply.github.com>

* fix(flow-item, panel): fix layout issue that would cause double scrollbars (#5486)

#5428

* 1.0.0-next.598

* docs: update component READMEs (#5501)

Co-authored-by: jcfranco <jcfranco@users.noreply.github.com>

* build(deps): bump eslint-plugin-jest from 27.1.1 to 27.1.2 (#5507)

Bumps [eslint-plugin-jest](https://github.com/jest-community/eslint-plugin-jest) from 27.1.1 to 27.1.2.
- [Release notes](https://github.com/jest-community/eslint-plugin-jest/releases)
- [Changelog](https://github.com/jest-community/eslint-plugin-jest/blob/main/CHANGELOG.md)
- [Commits](jest-community/eslint-plugin-jest@v27.1.1...v27.1.2)

---
updated-dependencies:
- dependency-name: eslint-plugin-jest
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump shell-quote from 1.7.3 to 1.7.4 (#5503)

Bumps [shell-quote](https://github.com/ljharb/shell-quote) from 1.7.3 to 1.7.4.
- [Release notes](https://github.com/ljharb/shell-quote/releases)
- [Changelog](https://github.com/ljharb/shell-quote/blob/main/CHANGELOG.md)
- [Commits](ljharb/shell-quote@1.7.3...v1.7.4)

---
updated-dependencies:
- dependency-name: shell-quote
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump @typescript-eslint/eslint-plugin from 5.38.1 to 5.40.1 (#5506)

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 5.38.1 to 5.40.1.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.40.1/packages/eslint-plugin)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/eslint-plugin"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump @typescript-eslint/parser from 5.38.1 to 5.40.1 (#5504)

Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 5.38.1 to 5.40.1.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.40.1/packages/parser)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/parser"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump eslint-plugin-jsdoc from 39.3.6 to 39.3.13 (#5505)

Bumps [eslint-plugin-jsdoc](https://github.com/gajus/eslint-plugin-jsdoc) from 39.3.6 to 39.3.13.
- [Release notes](https://github.com/gajus/eslint-plugin-jsdoc/releases)
- [Commits](gajus/eslint-plugin-jsdoc@v39.3.6...v39.3.13)

---
updated-dependencies:
- dependency-name: eslint-plugin-jsdoc
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(tree, tree-item): works when tree is the topmost element in a shadow root where it has no parent (#5472)

#5333

* ci: fix next releases take 4 (#5500)

* ci: fix next releases take 4

* separate error messages

Co-authored-by: Ben Elan <benelan@users.noreply.github.com>

* 1.0.0-next.599

* fix(flow-item): Render back button first. (#5511)

* 1.0.0-next.600

* fix(tab): applies section styles onto the enclosing parent (#5516)

* WIP: temorary demo swap with the issue case

* fix(tab): applies section styles onto the enclosing parent

* revert to original demo

* cleanup

* TabChilrenWithPercentageHeights story

* cleanup

* swap parent selector for [role=tabpanel]

* Introduce a new class selector

* 1.0.0-next.601

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Ben Elan <belan@esri.com>
Co-authored-by: Ben Elan <benelan@users.noreply.github.com>
Co-authored-by: Calcite Admin <calcite-admin@esri.com>
Co-authored-by: jcfranco <jcfranco@users.noreply.github.com>
Co-authored-by: Kitty Hurley <khurley@esri.com>
Co-authored-by: Anveshreddy mekala <anv11827@esri.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Eliza Khachatryan <eli97736@esri.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Matt Driscoll <mdriscoll@esri.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants