Skip to content

Conversation

@vdua
Copy link
Collaborator

@vdua vdua commented Aug 8, 2024

  • randomly chosen step as 0.01 for decimal, but it can be author defined

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes and the overall coverage did not decrease.
  • All unit tests pass on CircleCi.
  • I ran all tests locally and they pass.

- randomly chosen step as 0.01 for decimal, but it can be author defined
@vdua vdua requested a review from rismehta August 8, 2024 22:23
@vdua vdua marked this pull request as draft August 8, 2024 22:23
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
focus-order-semantics minor
label-title-only serious
landmark-one-main moderate
region moderate
target-size serious

1 similar comment
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
focus-order-semantics minor
label-title-only serious
landmark-one-main moderate
region moderate
target-size serious

@adobe-bot
Copy link

Lighthouse scores (mobile)

Performance Accessibility Best-Practices SEO
Scores 90 96 96 75

@adobe-bot
Copy link

Lighthouse scores (desktop)

Performance Accessibility Best-Practices SEO
Scores 100 96 96 75

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
focus-order-semantics minor
label-title-only serious
landmark-one-main moderate
region moderate
target-size serious

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

1 similar comment
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Lighthouse scores (mobile)

Performance Accessibility Best-Practices SEO
Scores 91 96 96 75

@adobe-bot
Copy link

Lighthouse scores (desktop)

Performance Accessibility Best-Practices SEO
Scores 100 96 96 75

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

2 similar comments
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Lighthouse scores (desktop)

Performance Accessibility Best-Practices SEO
Scores 100 96 96 75

@adobe-bot
Copy link

Lighthouse scores (mobile)

Performance Accessibility Best-Practices SEO
Scores 89 96 96 75

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

2 similar comments
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@rismehta rismehta marked this pull request as ready for review August 11, 2024 10:05
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
focus-order-semantics minor
label-title-only serious
landmark-one-main moderate
region moderate
target-size serious

@adobe-bot
Copy link

Lighthouse scores (desktop)

Performance Accessibility Best-Practices SEO
Scores 100 96 96 75

@adobe-bot
Copy link

Lighthouse scores (mobile)

Performance Accessibility Best-Practices SEO
Scores 91 96 96 75

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

1 similar comment
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

*/

@Version("5.5.4") // aligning this with release/650 since af2-rest-api is compiled with 5.2.0 in release/650
@Version("5.6.4") // aligning this with release/650 since af2-rest-api is compiled with 5.2.0 in release/650
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the comment doesn't make sense here.

if (maxInput.value.includes('.')) {
maxInput.value = Math.floor(maxInput.value);
maxInput.setAttribute('value', maxInput.value);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can we add a test case for this use case where changing from number to integer, changes the min/max constraint as well

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
focus-order-semantics minor
label-title-only serious
landmark-one-main moderate
region moderate
target-size serious

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Lighthouse scores (desktop)

Performance Accessibility Best-Practices SEO
Scores 100 96 96 75

@adobe-bot
Copy link

Lighthouse scores (mobile)

Performance Accessibility Best-Practices SEO
Scores 90 96 96 75

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

1 similar comment
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
focus-order-semantics minor
label-title-only serious
landmark-one-main moderate
region moderate
target-size serious

@adobe-bot
Copy link

Lighthouse scores (desktop)

Performance Accessibility Best-Practices SEO
Scores 100 96 96 75

@adobe-bot
Copy link

Lighthouse scores (mobile)

Performance Accessibility Best-Practices SEO
Scores 90 96 96 75

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

2 similar comments
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@codecov
Copy link

codecov bot commented Aug 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.94%. Comparing base (8256a6d) to head (7a9e477).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #1348      +/-   ##
============================================
+ Coverage     81.76%   81.94%   +0.17%     
- Complexity      881      899      +18     
============================================
  Files           101      103       +2     
  Lines          2314     2337      +23     
  Branches        314      317       +3     
============================================
+ Hits           1892     1915      +23     
  Misses          259      259              
  Partials        163      163              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
focus-order-semantics minor
label-title-only serious
landmark-one-main moderate
region moderate
target-size serious

@adobe-bot
Copy link

Lighthouse scores (mobile)

Performance Accessibility Best-Practices SEO
Scores 90 96 96 75

@adobe-bot
Copy link

Lighthouse scores (desktop)

Performance Accessibility Best-Practices SEO
Scores 100 96 96 75

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

2 similar comments
@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@adobe-bot
Copy link

Accessibility Violations Found

Id Impact
label-title-only serious
target-size serious

@rismehta
Copy link
Collaborator

Merging this, since @vdua has already reviewed the changes.

@rismehta rismehta merged commit 30da958 into dev Aug 12, 2024
@rismehta rismehta deleted the number-input-min-max-decimal branch August 12, 2024 13:10
dmaurya929 pushed a commit that referenced this pull request Aug 13, 2024
…al (#1348)

* Fixing Min/Max dialog fields for number input, when the type is decimal

- randomly chosen step as 0.01 for decimal, but it can be author defined

* fixing indentation

* the correct value for decimal type is number

* Adding support for decimal in minimum/maximum

* Addressing review comments and adding more test cases

* Removing skipped test

* Adding more test

---------

Co-authored-by: Rishi Mehta <rismehta@adobe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants