s00500 / ESPUI

A simple web user interface library for ESP32 and ESP8266
https://valencia.lbsfilm.at/midterm-presentation/
Other
923 stars 174 forks source link

Slider in group not changing style dynamically #173

Closed JD-GG closed 2 years ago

JD-GG commented 2 years ago

Bug When changing the style of a slider in a slider-group using setElementStyle(uint16_t id, String style) it won't change dynamically and you have to refresh the page to see the changes.

Works fine when slider is not in a slider-group.

To Reproduce Steps to reproduce the behavior:

  1. Make a slider-group
  2. Change the style of one slider with setElementStyle(uint16_t id, String style) during runtime (i used ESPUI.setElementStyle(saturationSliderId, "background: linear-gradient(90deg, #ffffff 0%, #"+hex+" 100%);");)
  3. See no change
  4. Refresh Page to see change

Expected behavior Slider should have changed appearance dynamically. Tried the same with a single slider and it changed color dynamically with the same function-call

Calling updateControl(uint16_t id) does not work and also already gets called in setElementStyle(uint16_t id, String style)

Desktop:

Additional context

iangray001 commented 2 years ago

Hmm, thanks, I'll have a look into this when I get a chance.

JD-GG commented 2 years ago

After further testing it seems to be the same reason as to why you can't dynamically update the visibility in a group. Same as visibility it gets updated after the page is refreshed. Note that the slider with the problem is not the Parent-Slider but a slider below that one. The top slider changes style dynamically without problems with the same function call.

themindfactory commented 2 years ago

I just left a comment with regards to adding custom JS by code, and in this case if it existed, people could write little work arounds and "inject" them, just a thought!

Thanks!

pcbbc commented 2 years ago

Also seeing a similar issue when a slider value, other than the first in a group, is updated. It would appear this is down to the following line in slider.js ...

var number = parents.attr("id").substring(2);

...which causes a JS exception in the receiving browser...

Uncaught TypeError: parents.attr(...) is undefined
    slider_move http://192.168.2.110/js/slider.js:11
    handleEvent http://192.168.2.110/js/controls.js:36
    start http://192.168.2.110/js/controls.js:51
    onload http://192.168.2.110/:1

This hacky fix works for me, but doesn't feel right...

if (parents.attr("id")) {
    var number = parents.attr("id").substring(2);
    if (send) websock.send("slvalue:" + slider_new_val + ":" + number);
}

Sorry, not enough experience of how all this hangs together. Advice anyone?

iangray001 commented 2 years ago

Hi all, I am having trouble replicating this. I augmented my massive test program with a button to dynamically style a slider from a slider group using setElementStyle.

void groupSliderStyleCallback(Control *sender, int type) {
    ESPUI.setElementStyle(groupsliders3, "background: red;");
}

It all seemed to work fine without a refresh.

Screenshot 2022-05-24 at 21 39 01

Can I verify that you are both using the latest version of the library, the version from this repo?

@pcbbc if you look at the latest version of the minified controls.js then you'll see that it doesn't match up with your stack trace. Specifically handleEvent is called on line 50 of the current version but apparently 51 in yours.

@JD-GG same question to you. I think this is all fixed on the current version.

themindfactory commented 2 years ago

I think the format it was looking for was sl:xx but was missing sl: I should have traced down why the sl: was missing rather then removing substring, been without power now 3 days from huge storm so can't do that right now

On Tue., May 24, 2022, 17:10 RichardS from ESP8266, < @.***> wrote:

Happened to me too, the string it looks like it was expecting was in the form slxx xx being a number but when I checked it was just xx so I removed substring(2) and it worked

On Tue., May 24, 2022, 16:55 Ian Gray, @.***> wrote:

Hi all, I am having trouble replicating this. I augmented my massive test program with a button to dynamically style a slider from a slider group using setElementStyle.

void groupSliderStyleCallback(Control *sender, int type) { ESPUI.setElementStyle(groupsliders3, "background: red;"); }

It all seemed to work fine without a refresh.

[image: Screenshot 2022-05-24 at 21 39 01] https://user-images.githubusercontent.com/7703247/170128492-90b0415d-b6a1-4773-9314-b1cb1c5cc582.png

Can I verify that you are both using the latest version of the library, the version from this repo?

@pcbbc https://github.com/pcbbc if you look at the latest version of the minified controls.js https://github.com/s00500/ESPUI/blob/master/data/js/controls.min.js then you'll see that it doesn't match up with your stack trace. Specifically handleEvent is called on line 50 of the current version but apparently 51 in yours.

@JD-GG https://github.com/JD-GG same question to you. I think this is all fixed on the current version.

— Reply to this email directly, view it on GitHub https://github.com/s00500/ESPUI/issues/173#issuecomment-1136422800, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBV22IZDQRCXDIYXPEL4FDVLU64ZANCNFSM5VFNNPCA . You are receiving this because you commented.Message ID: @.***>

themindfactory commented 2 years ago

Happened to me too, the string it looks like it was expecting was in the form slxx xx being a number but when I checked it was just xx so I removed substring(2) and it worked

On Tue., May 24, 2022, 16:55 Ian Gray, @.***> wrote:

Hi all, I am having trouble replicating this. I augmented my massive test program with a button to dynamically style a slider from a slider group using setElementStyle.

void groupSliderStyleCallback(Control *sender, int type) { ESPUI.setElementStyle(groupsliders3, "background: red;"); }

It all seemed to work fine without a refresh.

[image: Screenshot 2022-05-24 at 21 39 01] https://user-images.githubusercontent.com/7703247/170128492-90b0415d-b6a1-4773-9314-b1cb1c5cc582.png

Can I verify that you are both using the latest version of the library, the version from this repo?

@pcbbc https://github.com/pcbbc if you look at the latest version of the minified controls.js https://github.com/s00500/ESPUI/blob/master/data/js/controls.min.js then you'll see that it doesn't match up with your stack trace. Specifically handleEvent is called on line 50 of the current version but apparently 51 in yours.

@JD-GG https://github.com/JD-GG same question to you. I think this is all fixed on the current version.

— Reply to this email directly, view it on GitHub https://github.com/s00500/ESPUI/issues/173#issuecomment-1136422800, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBV22IZDQRCXDIYXPEL4FDVLU64ZANCNFSM5VFNNPCA . You are receiving this because you commented.Message ID: @.***>

iangray001 commented 2 years ago

There should be no need to touch slider.js. Can I just verify @themindfactory that you are using the current version from this repo?

i.e. not whatever is in the Arduino library manager thing?

pcbbc commented 2 years ago

@iangray001 Yes, using the latest version. Sorry, I'd just inserted a line of debug at line 50 when I copied the exception trace. My issue is slightly different from the bug report, so perhaps unrelated? It concerns changing the second slider in a group, and the dynamical updating of that slider in the browser of another client. That other client does not update the second slider. It throws the exception.

iangray001 commented 2 years ago

Ahh so you're saying the issue is the second slider updating in another browser. Sorry, I missed that. I'll test that tonight.

JD-GG commented 2 years ago

@iangray001 I am 100% using the latest version. I recreated your example on multiple ESP8266. The 2nd and 3rd sliders are not changing dynamically. I am getting the following error on all my ESPs (same one as on my sketch)

void bCallback(Control *sender, int type) {
  ESPUI.setElementStyle(sliderB, "background: red;");
}

myExample

What difference could there be between your and my tests? I can`t try it on a ESP32 because i don't own one.

pcbbc commented 2 years ago

Looks like the exact same stacktrace locations and error as my recreate.

themindfactory commented 2 years ago

Click on slider.js:11:609 and post the code underlined in red

On Fri., May 27, 2022, 20:54 pcbbc, @.***> wrote:

Looks like the exact same stacktrace locations and error as my recreate.

— Reply to this email directly, view it on GitHub https://github.com/s00500/ESPUI/issues/173#issuecomment-1140123075, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBV22NAKHOWF24N4AEGIV3VMFVC3ANCNFSM5VFNNPCA . You are receiving this because you were mentioned.Message ID: @.***>

pcbbc commented 2 years ago

See earlier post…. var number = parents.attr("id").substring(2);

themindfactory commented 2 years ago

Remove .substring(2)

On Fri., May 27, 2022, 23:42 pcbbc, @.***> wrote:

See earlier post…. var number = parents.attr("id").substring(2);

— Reply to this email directly, view it on GitHub https://github.com/s00500/ESPUI/issues/173#issuecomment-1140160564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBV22OS5WT6ZPQKKFGC4BTVMGIZVANCNFSM5VFNNPCA . You are receiving this because you were mentioned.Message ID: @.***>

themindfactory commented 2 years ago

You'll find parents ID to be a number starting at position 0 not position 2

On Sat., May 28, 2022, 00:38 RichardS from ESP8266, < @.***> wrote:

Remove .substring(2)

On Fri., May 27, 2022, 23:42 pcbbc, @.***> wrote:

See earlier post…. var number = parents.attr("id").substring(2);

— Reply to this email directly, view it on GitHub https://github.com/s00500/ESPUI/issues/173#issuecomment-1140160564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBV22OS5WT6ZPQKKFGC4BTVMGIZVANCNFSM5VFNNPCA . You are receiving this because you were mentioned.Message ID: @.***>

iangray001 commented 2 years ago

You probably shouldn't just cut out bits of slider.js. All ESPUI elements have IDs of the format "xxNN" and so if slider_move is ever being called on an element where this isn't the case then something higher level has gone wrong.

However I still can't replicate this. Check this video, shows values and styles synchronising across multiple browsers in grouped sliders. https://user-images.githubusercontent.com/7703247/170825624-8e0df856-a58f-41c8-9bce-b126dce55142.mov Am I missing something?

You're both saying that you are using "the latest version" but I said "the current version from this repo? i.e. not whatever is in the Arduino library manager thing?".

As in, have you downloaded/cloned the code from this repo into the location where your libraries are manually?

If you open your version of controls.js is it identical to this one?

JD-GG commented 2 years ago

As in, have you downloaded/cloned the code from this repo into the location where your libraries are manually?

Yes.

If you open your version of controls.js is it identical to this one?

I manually copied the contents just to be sure.

Click on slider.js:11:609 and post the code underlined in red

.substring(2);if(send)websock.send("slvalue:"+slider_new_val+":"+number);}}

iangray001 commented 2 years ago

I manually copied the contents just to be sure.

Well this wouldn't help because that is just the source file. It is this file that is actually served (or the minified version if written into the filesystem). (Also, if you're doing that you need to rewrite the filesystem.)

Were they identical beforehand? The last commit changed line 654 to return; from switch (type) if it helps. Although I guess you've overwritten it now...

I presume you are using Arduino or PlatformIO, so remove the ESPUI library using their respective library managers, and then you'll know that you are using the files from this repo.

JD-GG commented 2 years ago

Were they identical beforehand?

I believe they were (because i downloaded the repo about 2 months ago). Anyway i redownloaded everything and rewrote the FS. Still doesn`t work :D (same err)

iangray001 commented 2 years ago

Interesting. So have you looked at my video? Does that look like it should recreate the issue you're seeing?

If so, can you prepare a minimal reproducible example please?

JD-GG commented 2 years ago

Sure thing :)

auto groupbutton = ESPUI.addControl(Button, "Button Group", "Button A", Dark, grouptab, aCallback);
ESPUI.addControl(Button, "", "Button B", Alizarin, groupbutton, bCallback);
ESPUI.addControl(Button, "", "Button C", Alizarin, groupbutton, cCallback);

void aCallback(Control *sender, int type) {
  ESPUI.setElementStyle(groupsliders, "background: red;");
}
void bCallback(Control *sender, int type) {
  ESPUI.setElementStyle(sliderB, "background: red;");
}
void cCallback(Control *sender, int type) {
  ESPUI.setElementStyle(sliderC, "background: red;");
}

https://user-images.githubusercontent.com/104784838/170840165-3a325ca9-8c58-4e40-82d8-a9c31eab99d5.mp4

themindfactory commented 2 years ago

OK found the error!

I downloaded the repo just now and figured it out.

in control.js in the handleevent function

  case UPDATE_SLIDER:
    $("#sl" + data.id).attr("value", data.value)
    *>>>>* *slider_move($("#id" + data.id <http://data.id>),

data.value, "100", false); <<<<* if(data.hasOwnProperty('elementStyle')) { $("#sl" + data.id).attr("style", data.elementStyle); } break;

should be

  case UPDATE_SLIDER:
    $("#sl" + data.id).attr("value", data.value)
    >>>> *slider_move($("#sl" + data.id <http://data.id>), data.value,

"100", false); <<<<* if(data.hasOwnProperty('elementStyle')) { $("#sl" + data.id).attr("style", data.elementStyle); } break;

On Sat, May 28, 2022 at 3:32 PM JD-GG @.***> wrote:

Sure thing :)

auto groupbutton = ESPUI.addControl(Button, "Button Group", "Button A", Dark, grouptab, aCallback); ESPUI.addControl(Button, "", "Button B", Alizarin, groupbutton, bCallback); ESPUI.addControl(Button, "", "Button C", Alizarin, groupbutton, cCallback);

void aCallback(Control sender, int type) { ESPUI.setElementStyle(groupsliders, "background: red;"); } void bCallback(Control sender, int type) { ESPUI.setElementStyle(sliderB, "background: red;"); } void cCallback(Control *sender, int type) { ESPUI.setElementStyle(sliderC, "background: red;"); }

https://user-images.githubusercontent.com/104784838/170840165-3a325ca9-8c58-4e40-82d8-a9c31eab99d5.mp4

— Reply to this email directly, view it on GitHub https://github.com/s00500/ESPUI/issues/173#issuecomment-1140318764, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBV22JCCR3E6RF4UIVKY6DVMJYGBANCNFSM5VFNNPCA . You are receiving this because you were mentioned.Message ID: @.***>

iangray001 commented 2 years ago

Oh for fuck's sake...

Well I've worked out why I can't recreate either of your bugs. I fixed it back in February, on my development repo and forgot to create a pull request to push the fix back here. Sorry!

Yes @themindfactory you are right. That was the issue and (nearly) the fix. Ah well silly me. I'll actually get this commit merged in.

iangray001 commented 2 years ago

Thank you both for all your help, sorry I made everything take longer by being an idiot :)

pcbbc commented 2 years ago

Not at all. It’s not always easy! Thanks for the fix.

JD-GG commented 2 years ago

"It works on my machine ¯\_(ツ)_/¯" Thank you for your time :D