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

Number normalizer #250

Merged
merged 3 commits into from
Sep 1, 2022
Merged

Number normalizer #250

merged 3 commits into from
Sep 1, 2022

Conversation

hunterlan
Copy link
Contributor

@hunterlan hunterlan commented Sep 1, 2022

⏱️ Before you start

  • Have you checked if a similar PR has already been requested?
  • Have you built and ran the app?

↗️ Related/Fixed issues

📄 Description

Added number normalizer helper. It's static class, which can be used everywhere.
Also, I changed a bit repository overview.

💭 Motivation and Context

📸 Assets (if appropriate):

image

@Lamparter Lamparter self-requested a review September 1, 2022 18:00
Copy link
Collaborator

@Lamparter Lamparter left a comment

Choose a reason for hiding this comment

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

@hunterlan This is a bit fussy, but can you fix this?

@hunterlan
Copy link
Contributor Author

@hunterlan This is a bit fussy, but can you fix this?

no problem!

@Lamparter Lamparter changed the title Number localizer Number normalizer Sep 1, 2022
@Lamparter Lamparter merged commit a8c94ed into 0x5bfa:main Sep 1, 2022
@0x5bfa
Copy link
Owner

0x5bfa commented Sep 1, 2022

Already been merged tho, LGTM.

@0x5bfa
Copy link
Owner

0x5bfa commented Sep 1, 2022

I've meant to use library such as DataSizeUnits. But Our team member already merged and I will work on it.

@Lamparter Lamparter added the pr-triage/ready-to-merge This PR has been approved by an FH member. label Sep 2, 2022
@0x5bfa
Copy link
Owner

0x5bfa commented Sep 6, 2022

@DeveloperWOW64 We should've used something library for this such as 'DataSizeUnits'.

@hunterlan
Copy link
Contributor Author

@DeveloperWOW64 We should've used something library for this such as 'DataSizeUnits'.

Why? It's not necessary, current convertor is working well

@0x5bfa
Copy link
Owner

0x5bfa commented Sep 6, 2022

Yeah. Of course, Thank you for your contribution. But I didn't want to use helpers but converters. The reason why is easy to convert value on XAML code.

@0x5bfa
Copy link
Owner

0x5bfa commented Sep 6, 2022

It is not necessary to add observable strong properties for converted values. Converters on XAML is just able to be converted as you expected with no properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-triage/ready-to-merge This PR has been approved by an FH member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Number normalizer
3 participants