Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Basic UI slider skips first item update after using it to change an item #2695

Closed
wborn opened this issue Dec 22, 2016 · 11 comments
Closed

Basic UI slider skips first item update after using it to change an item #2695

wborn opened this issue Dec 22, 2016 · 11 comments

Comments

@wborn
Copy link
Contributor

wborn commented Dec 22, 2016

Basic UI sliders skip the first item update after you use them to change the state of an item.

I can reproduce this issue on the Demo as follows:

  1. Open Widget Overview in the Basic UI demo
  2. Change Dimmer to 100 in the Basic UI (move the slider all the way to the right)
  3. Enter in the console: smarthome:update DimmedLight 10
  4. Notice that the icon of Dimmer updates but the slider does not
  5. Enter in the console: smarthome:update DimmedLight 5
  6. Notice that the slider of Dimmer now does update

The EventStream in my browser, used by the Basic UI, does seem to receive the correct events. Also Basic UI does correctly update the icon. It can also be reproduced with the Blinds item by updating the DemoBlinds item in the console.

Reloading the browser at step 4 also makes Basic UI show the correct value.

I've tested this with:

  • openHAB 2.0-SNAPSHOT Build # 663
  • Ubuntu 16.04.1
  • Chrome 55.0.2883.87 (64-bit)
@vlad-ivanov-name
Copy link
Contributor

vlad-ivanov-name commented Mar 5, 2017

Many of the slider problems in BasicUI are caused by the slider implementation in the Material Design Lite library. Sliders are implemented by applying styles to a native input type=range element, and with these elements, every single browser behaves differently (see google/material-design-lite#1738). This requires applying a number of hacks and quirks to get somewhat consistent behaviour. And even then from time to time there are bugs like events not being fired when they should and vice-versa, layout issues and so on.

Google's Material Design Lite has now evolved into Material Components for web, which, ironically, doesn't have sliders as of now. So, to fix many slider-related issues, our only option is to do the following:

  1. Fork Google MDL
  2. Reimplement sliders — use HTML elements instead of native input
  3. Add the forked library as a submodule for BasicUI
  4. Integrate it in the build process — gel legal clearance for all the javascript libraries and stuff.

In short, this is why people hate webdev :)

@wborn
Copy link
Contributor Author

wborn commented May 1, 2017

@resetnow It seems there might be a slider (material-components/material-components-web#25) in Material Components for web soon. It is currently scheduled for implementation in iteration 22 (15-28 May).

Migrating from MDL to MDC-Web could be a more future proof option. :-) Although the migration guide notes it is still in an alpha state.

@vlad-ivanov-name
Copy link
Contributor

@wborn if I understand correctly, it also uses a lot of modern ECMAScript stuff, which​ doesn't have a broad browser support.

@vlad-ivanov-name
Copy link
Contributor

modern ECMAScript stuff

Upon a closer look, the modern stuff seems to be compiled to a more or less cross-browser Javascript. However, another thing I don't like about Material Components for web is that the code is way bigger than MDL. BasicUI in its current state is very lightweight client-side, and I'd like to keep it that way.

@triller-telekom
Copy link
Contributor

Back to the original problem of @wborn: I just tried to reproduce your problem with the latest ESH and openHAB master branch and I cannot reproduce it anymore.

The latest snapshot is working correctly too:
https://openhab.ci.cloudbees.com/job/openHAB-Distribution/

https://openhab.ci.cloudbees.com/job/openHAB-Distribution/lastSuccessfulBuild/artifact/distributions/openhab/target/openhab-2.1.0-SNAPSHOT.tar.gz

So can you please try it again with this snapshot?

@resetnow Do you want to create a separate issue for the new Material Components for web? Because I think this is unrelated to this issue.

@wborn
Copy link
Contributor Author

wborn commented May 10, 2017

@triller-telekom I've just retested the issue but it still remains when using the reproduction scenario. At step 4 the icon updates but the slider does not.

I've retested with:

  • openHAB 2.1.0-SNAPSHOT Build # 912
  • Ubuntu 16.04.2
  • Chrome 58.0.3029.110 (64-bit)

@triller-telekom
Copy link
Contributor

I just tried it again and it just works...

@wborn: Could you maybe provide a screencast where you download the latest snapshot, unpack it, start it, let the demo initialize and then open basic ui, change the slider and then enter the command?

@resetnow: I really cannot reproduce it, neither my colleagues, so could you please have a look and try to reproduce it?

@wborn
Copy link
Contributor Author

wborn commented May 11, 2017

@triller-telekom I've now retested the issue with openHAB 2.1.0-SNAPSHOT Build # 914 and the slider now works better but I'm still able to get it to skip updates. I think this is due your code changes in PR #3403 (May 9th)?

These changes were not yet part of Build # 912 because it had:

openhab> bundle:list|grep Basic
194 | Active   |  80 | 0.9.0.201705081821     | Eclipse SmartHome Basic UI, Fragments: 203
203 | Resolved |  80 | 2.1.0.201705081517     | openHAB Basic UI Fragment, Hosts: 194

Build # 914 does contain these changes:

openhab> bundle:list|grep Basic
194 | Active   |  80 | 0.9.0.201705111157     | Eclipse SmartHome Basic UI, Fragments: 203
203 | Resolved |  80 | 2.1.0.201705111215     | openHAB Basic UI Fragment, Hosts: 194

So my original reproduction scenario no longer applies. I'll try to figure out another reproduction scenario.

@vlad-ivanov-name
Copy link
Contributor

vlad-ivanov-name commented May 11, 2017

I was able to reproduce it: https://resetnow.ru/temp/esh-screencast-3.webm

At 0:13, according to console, DimmedLight receives command '10'. However, the slider position is clearly not at 10%, so it's possible this is a client-side problem.

@vlad-ivanov-name
Copy link
Contributor

vlad-ivanov-name commented May 11, 2017

I think I'm looking at a different problem, sorry. This one can be explained by this comment: https://github.com/eclipse/smarthome/blob/master/extensions/ui/org.eclipse.smarthome.ui.basic/web-src/smarthome.js#L1304

As for the one described by @wborn — I can't reproduce it too, neither in Chrome nor in Firefox.

@wborn
Copy link
Contributor Author

wborn commented May 11, 2017

The remaining problem I have looks exactly like the one in @resetnow 's screencast. Occasionally the slider misses an update. The original problem seems to be solved between build # 912 and # 914. So I think the issue can be closed now. 👍

@wborn wborn closed this as completed May 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants