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

computerFileSize(): allow comma as decimal-point #34425

Closed
wants to merge 1 commit into from

Conversation

rubo77
Copy link
Contributor

@rubo77 rubo77 commented Oct 4, 2022

This should fix the issue #18468 (Cannot set user quota with uneven values using locales that use a comma as decimal separator)

Signed-off-by: Ruben Barkow-Kuder rubo77@users.noreply.github.com

Signed-off-by: Ruben Barkow-Kuder <rubo77@users.noreply.github.com>
@rubo77
Copy link
Contributor Author

rubo77 commented Oct 4, 2022

maybe we should use this instead of replaceAll, which is not supported by oder browsers:

replace(new RegExp(",","gm"),".")

@szaimen szaimen added bug 3. to review Waiting for reviews labels Oct 4, 2022
@szaimen szaimen added this to the Nextcloud 26 milestone Oct 4, 2022
@szaimen szaimen requested review from a team, PVince81, artonge and skjnldsv and removed request for a team October 4, 2022 18:12
@szaimen
Copy link
Contributor

szaimen commented Oct 4, 2022

Replaceall should be fine IIRC https://caniuse.com/?search=replaceall

@@ -83,7 +83,7 @@ export default {
return null
}

const s = string.toLowerCase().trim()
const s = string.toLowerCase().replaceAll(',', '.').trim()
Copy link
Contributor

Choose a reason for hiding this comment

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

I still need to test this but what happens if you enter multiple dots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dots stay, only commas are replaced with dots

@rubo77
Copy link
Contributor Author

rubo77 commented Oct 5, 2022

Replaceall should be fine IIRC https://caniuse.com/?search=replaceall

Firefox 76 is only 2 years old: may 5, 2020.

I think, we should still support it

@rubo77
Copy link
Contributor Author

rubo77 commented Oct 5, 2022

Please test this with a German or French locale in your browser

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Thanks for your PR!

I just tested this and it seems to work on the first glance. However after reloading the page, only whole values are applied so this would need an additional fix somewhere else.

@rubo77
Copy link
Contributor Author

rubo77 commented Oct 14, 2022

@szaimen could you please take over this pr then? I have no idea, where to search for

@szaimen
Copy link
Contributor

szaimen commented Oct 17, 2022

@szaimen could you please take over this pr then? I have no idea, where to search for

I would like to but unfortunately I do not know where to look at either. Maybe someone of @nextcloud/server-frontend knows?

@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
@blizzz blizzz mentioned this pull request Nov 6, 2023
This was referenced Nov 10, 2023
@blizzz blizzz mentioned this pull request Nov 20, 2023
5 tasks
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv
Copy link
Member

As there is no feedback since a while I will close this ticket. If this is still happening please make sure to upgrade to the latest version. After that, feel free to reopen.

@skjnldsv skjnldsv closed this Feb 24, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Feb 24, 2024
@skjnldsv skjnldsv removed the 3. to review Waiting for reviews label Feb 24, 2024
@skjnldsv skjnldsv deleted the rubo77-computer-filesize branch March 14, 2024 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants