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

[MainUI] fix type "number" for list separator in Designer #950

Merged
merged 2 commits into from
Mar 10, 2021
Merged

[MainUI] fix type "number" for list separator in Designer #950

merged 2 commits into from
Mar 10, 2021

Conversation

DarkC35
Copy link
Contributor

@DarkC35 DarkC35 commented Mar 10, 2021

Fixes #949

After changing the separator type to text it works as intended but there was one problem in following use case: when you need a space ' ' as separator character/string, the updateParameter method interprets it as a non-NaN value (since isNaN(' ') is false) and resultet in a NaN as input string. (This also occurs on other text inputs when you type a space, like url, refresh, frequency, ...)

I tought about two ways to solve this:

  1. Only check for isNaN when type="number" (like in the second commit)
  2. use isNaN(parseFloat(value)) instead of isNaN(value), since parseFloat(' ') === NaN so the check would work. This way had one problem when the number of space characters is important because parseFloat(' 1') === 1 and this will trim leading whitespaces of the input.

DarkC35 added 2 commits March 10, 2021 16:08
Signed-off-by: Kevin Haunschmied <[email protected]>
@DarkC35 DarkC35 requested a review from a team as a code owner March 10, 2021 15:34
@DarkC35 DarkC35 changed the title 949 list separator [MainUI] fix type "number" for list separator in Designer Mar 10, 2021
@relativeci
Copy link

relativeci bot commented Mar 10, 2021

Job #18: Bundle Size — 10.88MB (~+0.01%).

ac6636d vs f4fc2a8

Changed metrics (1/8)
Metric Current Baseline
Cache Invalidation 0.84% 85.77%
Changed assets by type (1/7)
            Current     Baseline
JS 8.55MB (~+0.01%) 8.55MB

View Job #18 report on app.relative-ci.com

Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@ghys ghys merged commit 1050631 into openhab:main Mar 10, 2021
@ghys ghys added this to the 3.1 milestone Apr 1, 2021
@ghys ghys added bug Something isn't working main ui Main UI labels Apr 1, 2021
ghys pushed a commit that referenced this pull request Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MainUI - Sitemap Pagedesigner] Wrong type "number" for list separator in Designer
2 participants