toggledbits / Reactor

Reactor (for Vera and openLuup) is a Vera Home Automation plugin that provides advanced programmable logic.
18 stars 0 forks source link

Condition is sustained for "at least" <> seconds not editable with Safari Version 12.1 (14607.1.40.1.3) #15

Closed bblacey closed 5 years ago

bblacey commented 5 years ago

Using Safari Version 12.1 (14607.1.40.1.3) under macOS 10.14.4 Beta (18E215a), I am not able to edit the Condition sustained for field. I am running Reactor stable 32033c0 . I am able to edit the field using Chrome so this must be a Safari-only issue.

Also, apologies for the multi-updates - inadvertently hit the enter key which prematurely filed the issue.

toggledbits commented 5 years ago

Just to confirm, the repeat count field was empty when observing this behavior, correct?

bblacey commented 5 years ago

Yes. I even tried to force-reload w/cache-reset the page, etc. and I simply couldn't enter a value into that field. Launched Chrome, navigated to the Reactor sensor, clicked the chevron and entered a value.

bblacey commented 5 years ago

Perhaps it only manifests itself when creating a new sensor because that is what I did. I created a sensor and tried to configure it to perform an action if a device state as maintained for 15 minutes (e.g. lock the door if it was unlocked for 15 minutes).

toggledbits commented 5 years ago

I can't test on Safari because I don't own any Apple products (at least, none that I've paid for--I do have a donated iPhone from the community that I use for testing mobile app stuff). The code is fairly generic and simple, and it could simply be a compatibility issue between the ancient version of jQuery that UI7 uses and the newer Safari (or perhaps any Safari). I have no way of knowing. So I have to rely on you at the moment. You said

Perhaps it only manifests itself when creating a new sensor because that is what I did.

Can you go back and edit after the fact? Does it work on existing sensors?

bblacey commented 5 years ago

Hey Patrick, I can not edit the value in the field with Safari. I was able to enter it with Chrome and I see the value in Safari but when I click in the field, it basically ignores it. I'll look into it a bit more tomorrow.

toggledbits commented 5 years ago

This might be one of those cases where a screen shot may help, if you can.

bblacey commented 5 years ago

Agreed. Here is a short screencast that captures the issue and I hope helps illuminate the root cause.

Note: My response time is a bit slow recently because I haven't had as much computer time. Pretty much early mornings and late evenings EDT.

toggledbits commented 5 years ago

OK. If you enter a number into the "Condition repeats" field and tab out of it, the UI should enable the "within seconds" field to its right, and disable the "sustained for" field (gray). Then, if you remove the number from "Condition repeats" and tab out, it should disable "within seconds" again, and re-enable then "sustained for" field. At that point, are you able to edit in the "Sustained for" field?

bblacey commented 5 years ago

Well, sort of. If I enter a number >1 into Condition Repeats, the seconds field becomes editable and if I remove Condition repeats, I am able to remove the number and then tab back. During your test, I discovered it all devolves down to, I can edit the Condition is sustained for IFF I tab into the field. I can not click in the field with the mouse/cursor. All other fields behave as expected.

toggledbits commented 5 years ago

OK. That's good info. I was able to Google out some odd layering/click handling complaints that are similar, and restructure the HTML on a hunch based on that research.

Try the stable branch version of J_ReactorSensor_UI7.js... upload, hard-reload/refresh your browser. No Luup reload needed, though, just the browser refresh. Version should report 19079 in the Tools tab footer.

bblacey commented 5 years ago
(base) [blacey@bbl ~/Projects/Vera/Vera-Reactor (stable)]$ git pull
remote: Enumerating objects: 19, done.
remote: Counting objects: 100% (19/19), done.
remote: Compressing objects: 100% (3/3), done.
remote: Total 11 (delta 8), reused 11 (delta 8), pack-reused 0
Unpacking objects: 100% (11/11), done.
From https://github.com/toggledbits/Reactor
   32033c0..22c3e15  stable     -> origin/stable
   c1d19b7..0aa581e  develop    -> origin/develop
Updating 32033c0..22c3e15
Fast-forward
 J_ReactorSensor_UI7.js |  6 +++---
 J_Reactor_UI7.js       |  2 +-
 L_Reactor.lua          | 12 ++++++------
 3 files changed, 10 insertions(+), 10 deletions(-)
(base) [blacey@bbl ~/Projects/Vera/Vera-Reactor (stable)]$ uploadVeraLUUP.sh vera
OK|D_Reactor.xml
OK|D_ReactorDeviceInfo.json
OK|D_ReactorSensor.xml
OK|D_ReactorSensor_UI7.json
OK|D_Reactor_UI7.json
OK|I_Reactor.xml
OK|J_ReactorSensor_ALTUI.js
OK|J_ReactorSensor_UI7.js
OK|J_Reactor_ALTUI.js
OK|J_Reactor_UI7.js

And it worked perfectly! Well done!

toggledbits commented 5 years ago

Hey! Even a blind squirrel gets an acorn once in a while! :-)

OK. So the issue was that the select element and input element were surrounded by a label element. This should not be a problem, but apparently on Safari they do something weird with the labels in an attempt to make them clickable, so that clicking on the label is the same as clicking on the related field (perhaps, as you would expect for a checkbox: you can click on the text and it toggles the checkbox). In any case, whatever they are doing is causing the label element to "steal" the click from the input, so as you indicated, you can edit it if you tab into it, but you can't click on it directly to give it focus.

All I did was restructure the HTML there so that the labels are not containers for the select and input, and it apparently works. Odd, but not likely to cause any side-effects elsewhere, so fine.

FWIW, there are other input fields wrapped with a label that work fine (the repeats, etc). So it appears to be somehow related to the presence of the select with the input inside the same label, as this was the only place where that was done.

bblacey commented 5 years ago

Awesome! Fix confirmed in 22c3e15