FlowFuse / node-red-dashboard

https://dashboard.flowfuse.com
Apache License 2.0
204 stars 49 forks source link

Text Input Node not sending msg.payload on focus leave (0.10.2 bug / regression) #442

Closed thebaldgeek closed 9 months ago

thebaldgeek commented 11 months ago

Description

Previous version of the v2 dash Text Input respected the 'Focus Leave' check box. This version does not. Also this version and all previous tested versions does not sticky the unchecking of the Delay check box. After a deploy the box is checked again. My work around is to put a very large delay in there.

Currently the only way to get data out of the Text input node is to check the 'Press Enter' or wait for the delay to time out. It is unintuitive to the dashboard user that they need to press enter for each text box on the page before the page will function.

Node-RED 3.1.0 and nodeJS 18.9.0

Properties

No response

Events

No response

Controls

No response

Existing Examples

No response

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

colinl commented 11 months ago

I can confirm that I also see both issues: Focus leave not working and Delay gets re-checked automatically.

joepavitt commented 10 months ago

Focus leave not working

Have a fix for this coming

Delay gets re-checked automatically.

Different issue, but this is because I assume that if a number is present, then the tick gets re-activated. Happy to remove that assumption, and give control on the checkbox entirely to user.

joepavitt commented 10 months ago

With this, I also block the onBlur if the value matches the last value sent, i.e. if your'e tabbing through inputs, you won't want it to re-send everything if you're making no changes. Similarly, if the value was sent as a consequence of onEnter or onDelay, then you don't want to send it again when you focus away from that input

thebaldgeek commented 10 months ago

Ok, can confirm that blanking out the time does now retain the check box (very unclear that was an option). I have just the one check box now, 'Focus Leave' and am not getting anything on the debug node attached, ie, on focus leave, the entered text is not sent. I will try and make a very small flow to show how I have 3 text boxes and one button. The thought/plan was simple, user enters text in 1 or more boxes and clicking the button will force focus leave and send all text boxes at that time. Its not working in v2, works a treat in v1 (not trying to move v1 to v2, given up on that, starting whole new fresh v2 site).

joepavitt commented 10 months ago

Fix is imminent :)

colinl commented 10 months ago

I assume that if a number is present, then the tick gets re-activated.

I don't think that is expected. If the user unticks the box then that it should be disabled. Ideally the field would grey out when deactivated, but that is a minor issue.

joepavitt commented 10 months ago

If the user unticks the box then that it should be disabled

This is now the way - release should be out later, trying to tick off a few smaller issues this afternoon

colinl commented 10 months ago

trying to tick off a few smaller issues this afternoon

If you are looking for some that would be useful you could look at 352, 311 and 396 :)

thebaldgeek commented 10 months ago

I'm now about 2 hours into trying to get the text node to consistently send a value now that the delay option is fixed... Using v0.11.1 Sorry to bug you @joepavitt but I could use some input on what you mean by the 'Focus Leave' check box.... I have a a few text boxes and here is what I expected, but is not happening....

image

You can see 'wifi' in the box as some sample test, the cursor is in there as well (focus not left), if I click on another box or click the search button I expected that to mean a 'Focus Leave' event and there should be an output from that text box in the debug tab, but there is none. Best as I can tell is that it works just the once after a deploy. If you try it any number of times after that, it does not detect the 'Focus Leave' event?

BTW, I have a 4 second delay that sends msg.payload = ""; via function node to all the text boxes so the user gets a fresh start after clicking the search button. This 'blank' string is not sent out as I don't want to then fire off a blank search. Just clear any used text boxes.

image

image

joepavitt commented 10 months ago

You can see 'wifi' in the box as some sample test, the cursor is in there as well (focus not left), if I click on another box or click the search button I expected that to mean a 'Focus Leave' event and there should be an output from that text box in the debug tab, but there is none

You're right. That should be happening.

I had built it in so that it never resends the same value. So if you type "Wifi", tab out, it should send the message.

Then, if you re-focussed it, and tabbed out without changing the value, it would not resend anything.

joepavitt commented 10 months ago

Appreciate the time in trying to get it to work. Always open to feedback and suggestions on behaviours here.

The difficulty in having it always send on blur (focus leave) is if its also done with delay/onEnter, then you end up with same value being sent multiple times.

On reflection, I should probably be leaving that to the user, up to the user which options to enable, and always send a message onBlur if that's what's been configured?

joepavitt commented 10 months ago

@thebaldgeek if you'd prefer it to send on every blur - let me know, and we'll make the change. Also open to PRs of you're that way inclined, I have limited access to laptop over the next week

thebaldgeek commented 10 months ago

Ahhhh. I think that's what's been happening. I have been testing your code with my code and so I have been putting wifi in the first and then inop and reset in the other two search options and trying differing combos of my second two terms. So that means I have been typing wifi over and over in the first thinking my payload = ''; would wipe all traces, but seems that your 'its the same string, don't send it' code has been seeing the same wifi string.... No wonder Im going a little bald figuring out what's happening. I bet the deploy is what's 'resetting' your same value code.... Ok, starting to make sense.

Thanks for the quick reply on the weekend too BTW.

Trying to think of the other use cases I have been using for the text node and what others might expect to be the given behaver.... I think if the check box says focus leave, it should mean focus leave even if its the same value. ie, the extra bit of 'hidden code' that was saying - not sending since its the same - was really doing my head in.

Ok, thinking more about your point of delay/onEnter.. I now see your challenge.... Hmm, ok, lets not rush into this, but thanks again for the quick reply. I can keep working on things now understanding how the hidden code was interacting.

BTW, on / off topic. I had a 3 hour! (a record for duration and attendance) live stream yesterday showing off the v2 vs v1 sites as they currently stand. There was much elation about the responsive (coming soon) tables and the clearer layout etc. The fleshed out text search had every one very very excited. I could not show it working, but they got the idea. My point is, a TON of people are looking forward to using it when it goes live.

thebaldgeek commented 9 months ago

Testing 'on blur' (focus leave) with 0.11.6 and it seems to be the same (as 0.11.0)?

Looking to have, as you said: On reflection, I should probably be leaving that to the user, up to the user which options to enable, and always send a message onBlur if that's what's been configured?

joepavitt commented 9 months ago

Hmm, that's very bizarre. I've not touched ui-text-input since then.

Behaviour should be as you expect. I haven't yet managed to get the E2E tests written for it though, will prioritise on Tuesday when I'm back working.

joepavitt commented 9 months ago

Just tried out the following variants and they're working for me atm:

https://github.com/FlowFuse/node-red-dashboard/assets/99246719/ef72f730-ce50-44c2-ba93-3ab212b47452

Do you have a screen recording or flows.json I can try out?

thebaldgeek commented 9 months ago

Sorry for not better describing the issue.... Yes, the onBlur is working, so we are working MUCH better there. Thank-you so much for that fix!

The problem that has not changed is using the same text more than once.

image

If someone searches for both 'wifi' and 'reset' but simply uses different aircraft types each time, since the first two text words are the same in each search, the msg.payload for those two UI text boxes are returned empty.

A sample flow would be a text ui node set for 'Press Enter' or 'Focus Leave' and a debug node. Type different words in and you see them in the debug, type the same and nothing happens.

I guess I need to keep some sort of flow.context and 'unfake' your hidden 'deduplicate' code in my flow....

joepavitt commented 9 months ago

Thanks for the clarity - need to look into a ui-template issue as a priority tomorrow, but will make this second on the list!

joepavitt commented 9 months ago

Fix is in - not in a release yet though

thebaldgeek commented 9 months ago

Fantastic! Thanks so much. Happy to wait knowing that its getting close to where I can start to use the new V2 stuff beyond just some light testing.

joepavitt commented 9 months ago

Closed (again) by #507