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

Added ES6 local support for formatNumber helper as per #2951 #3099

Merged
merged 6 commits into from
Oct 13, 2021
Merged

Added ES6 local support for formatNumber helper as per #2951 #3099

merged 6 commits into from
Oct 13, 2021

Conversation

Braunson
Copy link
Contributor

@Braunson Braunson commented Oct 8, 2021

#2951

Changes proposed in this pull request:

Updated formatNumber helper to use ES6 method to format numbers via provided locale. Unsure how to grab the current user's locale to inject when the helper is used in PostStreamScrubber.js. Would app.session.user.locale work if it's not necessarily set?

Confirmed

  • Frontend change

@yibum
Copy link

yibum commented Oct 8, 2021

app.data.locale might work.
I have similar approach several days ago master...yibum:xm/format-number-locale
but I have not yet tested it.

@SychO9
Copy link
Member

SychO9 commented Oct 8, 2021

You can indeed use app.data.locale which is the current user's locale.

@Braunson
Copy link
Contributor Author

Braunson commented Oct 8, 2021

Updated to support the user's local and make the locale parameter optional.

davwheat
davwheat requested changes Oct 8, 2021
View reviewed changes
js/dist-typings/common/utils/formatNumber.d.ts Outdated Show resolved Hide resolved
js/src/common/utils/formatNumber.ts Outdated Show resolved Hide resolved
@Braunson Braunson requested a review from davwheat October 8, 2021 14:37
@Braunson Braunson requested a review from davwheat October 8, 2021 14:45
askvortsov1
askvortsov1 approved these changes Oct 8, 2021
View reviewed changes
Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Thank you very much! We'll merge this after we release Flarum v1.1

@askvortsov1 askvortsov1 linked an issue Oct 11, 2021 that may be closed by this pull request
formatNumber() ignores current language convention while formatting numbers #2951
Closed
@askvortsov1 askvortsov1 merged commit 2a86c25 into flarum:master Oct 13, 2021
SychO9 added a commit that referenced this pull request Oct 14, 2021
@SychO9
chore: Import app from common app instead
d4c5973 
Introduced in #3099
@SychO9 SychO9 mentioned this pull request Oct 14, 2021
Merged
1 task
davwheat pushed a commit that referenced this pull request Oct 14, 2021
@SychO9
fix: import app from common app instead (#3104)
713d95e 
Introduced in #3099
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tankerkiller125 tankerkiller125 tankerkiller125 approved these changes

@askvortsov1 askvortsov1 askvortsov1 approved these changes

@davwheat davwheat Awaiting requested review from davwheat

Assignees
No one assigned
Labels
hacktoberfest-accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

formatNumber() ignores current language convention while formatting numbers
6 participants
@Braunson @yibum @SychO9 @tankerkiller125 @davwheat @askvortsov1