Skip to content
This repository has been archived by the owner. It is now read-only.

Basic UI: fix refresh of hidden/visible image/chart #4534

Merged
merged 1 commit into from Nov 20, 2017
Merged

Basic UI: fix refresh of hidden/visible image/chart #4534

merged 1 commit into from Nov 20, 2017

Conversation

@lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Nov 12, 2017

Fixes #4492
Fixes #4507
Ignore refresh when inappropriate

Signed-off-by: Laurent Garnier lg.hc@free.fr

@lolodomo
Copy link
Contributor Author

@lolodomo lolodomo commented Nov 12, 2017

@resetnow : please confirm that I don't need anymore to provide the file smarthome.js in the web directory but only the one in the web-src directory ?

If you can review, that would be cool.

@resetnow
Copy link
Contributor

@resetnow resetnow commented Nov 13, 2017

I don't need anymore to provide the file smarthome.js in the web directory

That's correct.

If you can review, that would be cool.

Later today.

Copy link
Contributor

@resetnow resetnow left a comment

I would also check if the browser actually makes requests for images that are hidden with display: none;.

_t.visible = state;
};

_t.disactivateRefresh = function() {
Copy link
Contributor

@resetnow resetnow Nov 13, 2017

You probably meant deactivateRefresh

Copy link
Contributor Author

@lolodomo lolodomo Nov 13, 2017

Yes

_t.disactivateRefresh = function() {
if (interval !== undefined) {
clearInterval(interval);
interval = undefined;
Copy link
Contributor

@resetnow resetnow Nov 13, 2017

JS type are weird but I think here null is more suitable as it's used in the rest of the code

Copy link
Contributor Author

@lolodomo lolodomo Nov 13, 2017

Ok I will change that.

String url = chartUrl + "&t=" + (new Date()).getTime();
String url;
if (!itemUIRegistry.getVisiblity(w)) {
url = "images/none.png";
Copy link
Contributor

@resetnow resetnow Nov 13, 2017

Feels a bit weird here, should be a static const maybe?

Copy link
Contributor Author

@lolodomo lolodomo Nov 13, 2017

Ok, why not.

interval = setInterval(function() {
if (_t.image.clientWidth === 0) {
clearInterval(interval);
return;
}
_t.image.setAttribute("src", _t.url + "&t=" + Date.now());
if (_t.image.getAttribute("src").startsWith(_t.url)) {
Copy link
Contributor

@resetnow resetnow Nov 13, 2017

Couldn't figure out what this is for.

Copy link
Contributor Author

@lolodomo lolodomo Nov 13, 2017

It is to avoid a refresh of the image using the proxied URL when the image was set with a raw data for example.

Copy link
Contributor Author

@lolodomo lolodomo Nov 13, 2017

Maybe I should rather set a variable to store the information whether we should refresh or not.

@lolodomo
Copy link
Contributor Author

@lolodomo lolodomo commented Nov 14, 2017

@resetnow : I have taken into account all your comments except one that remains to be considered.

Fixes #4492
Fixes #4507
Ignore refresh when inappropriate

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor Author

@lolodomo lolodomo commented Nov 15, 2017

@resetnow : everything is now ok and working well.

@kaikreuzer
Copy link
Contributor

@kaikreuzer kaikreuzer commented Nov 17, 2017

@resetnow If you can approve the changes, we should be good to merge!

Copy link
Contributor

@resetnow resetnow left a comment

LGTM

@maggu2810 maggu2810 merged commit d3301aa into eclipse-archived:master Nov 20, 2017
2 checks passed
@lolodomo lolodomo deleted the hidden_chart branch Nov 20, 2017
@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017
@kaikreuzer kaikreuzer added the bug label Dec 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants