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

Scenes - decimals in the "continue only if" block #1683

Merged
merged 3 commits into from
Jan 20, 2023

Conversation

VonOx
Copy link
Contributor

@VonOx VonOx commented Jan 9, 2023

FIX #1585

image

@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Base: 97.27% // Head: 97.27% // No change to project coverage 👍

Coverage data is based on head (071e7e4) compared to base (3d2ffc8).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1683   +/-   ##
=======================================
  Coverage   97.27%   97.27%           
=======================================
  Files         643      643           
  Lines        9686     9686           
=======================================
  Hits         9422     9422           
  Misses        264      264           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@atrovato atrovato left a comment

Choose a reason for hiding this comment

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

Some comments here 😉

@VonOx
Copy link
Contributor Author

VonOx commented Jan 10, 2023

Some comments here 😉

Thank you Alex, I will make the change.

I have also changed the event to onBlur ( lost focus) because on change with actual if statement prevent decimal input (dot). With only parsefloat I can revert to onChange, I don't know what is the best.

@VonOx VonOx requested a review from atrovato January 11, 2023 07:22
@atrovato
Copy link
Contributor

atrovato commented Jan 11, 2023

onBlur forces the user to leave the field for updates to be taken into account.
So if the user uses "enter" key on keyboard without leaving the field, changes on the filed will be avoid.
@VonOx

@VonOx
Copy link
Contributor Author

VonOx commented Jan 13, 2023

@atrovato I've tested with "enter key" and focus is not lost and even if I hit save button on scene, new value is handled.
onBlur is working great here.


onChange

parseFloat(1.) = 1 => impossible to input decimal separator

onChange handle need to implement this kind of condition to prevent the behavior.

if (e.target.value.charAt(str.length - 1) === '.') {
  return;
}

But In that condition we need need an input validator because if user set 1. and nothing else , new value is not handled.

@atrovato
Copy link
Contributor

Do I understand everything is alright?

@VonOx
Copy link
Contributor Author

VonOx commented Jan 13, 2023

Do I understand everything is alright?

With onBlur => Yes

Copy link
Contributor

@atrovato atrovato left a comment

Choose a reason for hiding this comment

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

Ok :)

Copy link
Contributor

@Pierre-Gilles Pierre-Gilles left a comment

Choose a reason for hiding this comment

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

EDIT: Deleting my previous message, it works!! Issue on my side :)

Copy link
Contributor

@Pierre-Gilles Pierre-Gilles left a comment

Choose a reason for hiding this comment

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

Good for me, it works fine in the UI and it works fine with a real device / real scene!

Thanks for this PR 🙏

PS: forget my previous review, I messed up my test ^^

@Pierre-Gilles Pierre-Gilles merged commit c553ec1 into GladysAssistant:master Jan 20, 2023
@relativeci
Copy link

relativeci bot commented Jan 20, 2023

Job #995: Bundle Size — 7.01MiB (~-0.01%).

c553ec1(current) vs d5d81dd master#994(baseline)

Metrics (2 changes)
                 Current
Job #995
     Baseline
Job #994
Initial JS 3.03MiB(~-0.01%) 3.03MiB
Initial CSS 294.78KiB 294.78KiB
Cache Invalidation 43.1% 59.06%
Chunks 52 52
Assets 148 148
Modules 1258 1258
Duplicate Modules 2 2
Duplicate Code 0.03% 0.03%
Packages 111 111
Duplicate Packages 8 8
Total size by type (1 change)
                 Current
Job #995
     Baseline
Job #994
CSS 312.43KiB 312.43KiB
Fonts 93.55KiB 93.55KiB
HTML 13.58KiB 13.58KiB
IMG 1.64MiB 1.64MiB
JS 4.95MiB (~-0.01%) 4.95MiB
Media 0B 0B
Other 4.95KiB 4.95KiB

View job #995 reportView master branch activity

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.

Use of decimals in the "continue only if" block
3 participants