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

[basicui] Use sitemap input hint v2 #1923

Merged
merged 25 commits into from
Jul 14, 2023

Conversation

mherwege
Copy link
Contributor

@mherwege mherwege commented Jun 13, 2023

This PR is a continuation of #1787, using the extra functionality provided by core if openhab/openhab-core#3644 gets merged in core. It would be an alternative to #1787 with the same functionality, but reduced complexity.

Requires openhab/openhab-core#3644.

mherwege added 23 commits July 2, 2023 15:14
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
@mherwege mherwege force-pushed the sitemap_input_hint_v2 branch from 3fa063d to b363e09 Compare July 2, 2023 13:17
@lolodomo lolodomo removed the awaiting other PR Depends on another PR label Jul 2, 2023
@lolodomo
Copy link
Contributor

lolodomo commented Jul 9, 2023

@mherwege : for your information, I have done tests for number and I discovered 2 issues:

  1. it does not work at all with number without dimension, the entered value is apparently rejected
  2. If I clear the field and press enter, a value is restored but not the current value, rather an old value, probably the value when the page was first opened.

For number with dimension, except problem 2, it seems to work well.

For problem 1, as it was working well before, I assume there is a link with the change you did in core framework. The problem is in the JavaScript as I see no command sent to the server in the event log.

I will try to solve these 2 issues.

@mherwege
Copy link
Contributor Author

mherwege commented Jul 9, 2023

The java script code was not touched in 3d6aa48, the commit adapting to the change in core. So both problems may already have been there with the other PR, or caused by the change in core.

@lolodomo
Copy link
Contributor

lolodomo commented Jul 9, 2023

The java script code was not touched in 3d6aa48, the commit adapting to the change in core. So both problems may already have been there with the other PR, or caused by the change in core.

But in fact, I did not test recently to fill a value.
The problem should be in parseNumber method, I assume.

@lolodomo
Copy link
Contributor

lolodomo commented Jul 9, 2023

The problem should be in parseNumber method, I assume.

But this method works for number with dimension so I am not sure. That is strange.

@lolodomo
Copy link
Contributor

lolodomo commented Jul 11, 2023

I am not able to reproduce problem 2 (bad value restored) now !

@mherwege
Copy link
Contributor Author

Make sure to first change it to another good value and only then try clearing the field. If there is an issue, it may be after a first successful update.

@lolodomo
Copy link
Contributor

I just made more tests for numbers whatever the hint and that looks good.
I discovered that when input hint is number, you can use your mouse scroll to increase/decrease the value and it works.
I just found one issue when input hint is number: using the "+" like for example +12 is not possible, the value is rejected. I propose that this issue is fixed later in another PR.

I did not test again date/time hints but I made intensive tests and feedback few months ago on that part.

Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo lolodomo merged commit 16256c2 into openhab:main Jul 14, 2023
@lolodomo lolodomo added this to the 4.0 milestone Jul 14, 2023
@mherwege
Copy link
Contributor Author

Thank you for all your review and adjustment effort.

@lolodomo
Copy link
Contributor

Thank you to you first :)

I opened an issue for the remaining (minor) issue. I will try to fix it next week. I should check if this is specific to Firefox or not.

@mherwege mherwege deleted the sitemap_input_hint_v2 branch July 28, 2023 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
basic ui Basic UI enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants