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

44702 Thousand separator in form inputs #758

Merged
merged 13 commits into from
Jul 18, 2024
Merged

44702 Thousand separator in form inputs #758

merged 13 commits into from
Jul 18, 2024

Conversation

dai-eastgate
Copy link
Contributor

@dai-eastgate dai-eastgate commented Feb 23, 2024 •
edited
Loading

related to #721
changed log:
Add thousand separator in form inputs

@dai-eastgate dai-eastgate changed the title 44702 thousand separator in form inputs 44702 Thousand separator in form inputs Feb 23, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 23, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9575953031

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 20 of 40 (50.0%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 79.507%
Changes Missing Coverage Covered Lines Changed/Added Lines %
plugin/Controller/InputVariableReaderFormatter.php 6 7 85.71%
plugin/EstateList.php 1 2 50.0%
plugin/Form.php 0 1 0.0%
plugin/Form/FormFieldValidator.php 3 7 42.86%
plugin/ScriptLoader/ScriptLoaderGenericConfigurationDefault.php 0 13 0.0%
Totals Coverage Status
Change from base Build 9563894875: -0.1%
Covered Lines: 8357
Relevant Lines: 10511
💛 - Coveralls

@fredericalpers fredericalpers linked an issue Feb 23, 2024 that may be closed by this pull request
Thousand separator in form inputs #721
Closed
4 tasks
@fredericalpers fredericalpers added this to the v4.21 milestone Feb 23, 2024
@dai-eastgate dai-eastgate force-pushed the 44702-thousand-separator-in-form-inputs branch from a7230ba to c3d7a73 Compare February 27, 2024 04:50
@dai-eastgate dai-eastgate force-pushed the 44702-thousand-separator-in-form-inputs branch from 9a2cc95 to cbcf420 Compare February 27, 2024 07:56
@fredericalpers fredericalpers added QA Issue or Pull request that is in review UI/UX Issue, Pull Request or Discussion related to UI/UX labels Mar 19, 2024
@fredericalpers fredericalpers self-assigned this Jun 27, 2024
@fredericalpers fredericalpers added the deploy test Triggers event to create test version label Jun 28, 2024
@github-actions GitHub Actions
Copy link

Steps to install the approved version:

  1. Download onoffice-4.20-12-gf403b19a-please-unpack.zip from https://github.com/onOffice-Web-Org/oo-wp-plugin/actions/runs/9709579448.
  2. Unpack the downloaded file to get another .zip file.
  3. Upload that inner .zip file to WordPress.

andernath
andernath reviewed Jul 9, 2024
View reviewed changes
Copy link
Contributor

@andernath andernath left a comment

Choose a reason for hiding this comment

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

@dai-eastgate looks good. I just have one question, could you take a look?

js/onoffice-apply-thousand-separator.js Outdated

const getSeparators = (separatorFormat) => ({
thousandSeparator: separatorFormat === 'comma-separator' ? ',' : '.',
decimalSeparator: separatorFormat === 'comma-separator' ? '.' : ','
Copy link
Contributor

Choose a reason for hiding this comment

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

In #721 (comment) you mention that you did not implement decimal separator.
Does onoffice-apply-thousand-separator.js need some refactoring ?

@dai-eastgate
Copy link
Contributor Author

@fredericalpers @andernath I removed the decimal separator implementation
=> Currently the thousands separator feature will be automatic, users cannot enter "," or "."
Ex:
With dot: 1000 -> 1.000; 123456 -> 123.456
With comma: 1000 -> 1,000; 123456 -> 123,456

Note: I have added a "Please choose" option for old users who are not using this feature

You can watch the demo video below:

758_EstateListSearch.mp4

@miauzjpg miauzjpg added deploy test Triggers event to create test version and removed deploy test Triggers event to create test version labels Jul 16, 2024
@github-actions GitHub Actions
Copy link

Steps to install the approved version:

  1. Download onoffice-4.20-19-g062651cf-please-unpack.zip from https://github.com/onOffice-Web-Org/oo-wp-plugin/actions/runs/9952134566.
  2. Unpack the downloaded file to get another .zip file.
  3. Upload that inner .zip file to WordPress.

miauzjpg
miauzjpg approved these changes Jul 18, 2024
View reviewed changes
Copy link
Contributor

@miauzjpg miauzjpg left a comment

Choose a reason for hiding this comment

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

great work!

@github-actions GitHub Actions
Copy link

Steps to install the approved version:

  1. Download onoffice-4.20-19-g062651cf-please-unpack.zip from https://github.com/onOffice-Web-Org/oo-wp-plugin/actions/runs/9986970956.
  2. Unpack the downloaded file to get another .zip file.
  3. Upload that inner .zip file to WordPress.

@miauzjpg miauzjpg merged commit 03c55fe into master Jul 18, 2024
9 checks passed
@miauzjpg miauzjpg deleted the 44702-thousand-separator-in-form-inputs branch July 18, 2024 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andernath andernath andernath left review comments

@miauzjpg miauzjpg miauzjpg approved these changes

Assignees

@fredericalpers fredericalpers

Labels
deploy test Triggers event to create test version QA Issue or Pull request that is in review UI/UX Issue, Pull Request or Discussion related to UI/UX
Projects
None yet
Milestone
v5.0
Development

Successfully merging this pull request may close these issues.

Thousand separator in form inputs
4 participants
@dai-eastgate @andernath @miauzjpg @fredericalpers