Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typedInput change event incorrect value #2883

Closed
5 tasks
Zehir opened this issue Feb 23, 2021 · 9 comments
Closed
5 tasks

typedInput change event incorrect value #2883

Zehir opened this issue Feb 23, 2021 · 9 comments
Labels

Comments

@Zehir
Copy link

Zehir commented Feb 23, 2021

What are the steps to reproduce?

Create a new node type by using this code;
In the html node definition set this code;

<script type="text/html" data-template-name="test-input">

    <div class="form-row">
        <label for="node-input-name" class="l-width"><i class="icon-tag"></i> <span
                data-i18n="label.name"></span></label>
        <input type="text" id="node-input-name" data-i18n="[placeholder]placeholder.name">
    </div>

    <div class="form-row">
        <label for="node-input-query" class="l-width"><i class="fa fa-search"></i> <span
                data-i18n="label.query"></span></label>
        <input type="text" id="node-input-query">
        <input type="hidden" id="node-input-search_type">
    </div>

</script>

<script type='text/javascript'>
    RED.nodes.registerType('test-input', {
        category: 'sample',
        color: '#f7aa3f',
        defaults: {
            name: {
                value: ""
            },
            search_type: {
                value: "device_list",
                required: true
            },
            query: {
                value: "{}",
                required: false
            },
        },
        inputs: 0,
        outputs: 1,
        outputLabels: ["state"],
        paletteLabel: 'in',
        icon: "icon.png",
        label: "test",
        oneditprepare: function () {
            let node = this;
            let querySelect = $("#node-input-query");

            console.log(node.search_type) // This return undefined probably because the node was saved once before adding the value in the defaults object.
            node.search_type = node.search_type || "device";

            querySelect.typedInput({
                type: "text",
                types: [{
                    value: "device",
                    label: "Device",
                    icon: "icons/node-red-contrib-deconz/deconz.png ",
                    hasValue: false
                }, "json", "jsonata"],
                typeField: "#node-input-search_type"
            })

            querySelect.on('change', function (type, value) {
                console.log({e: "onchange", type: type, value: value})
                console.log(querySelect.typedInput('type'))
            });

        }
    });
</script>

Then run node-red and open the node settings.

What happens?

In the console;

console.log({e: "onchange", type: type, value: value}) // Object {e: "onchange", type: S.Event, value: true}
console.log(querySelect.typedInput('type')) // device

What do you expect to happen?

In the console;

console.log({e: "onchange", type: type, value: value}) // Object {e: "onchange", type: S.Event, value: "device"}
console.log(querySelect.typedInput('type')) // device

Side note

If I put the on.change() before typedInput() I get 2 events, the first one is correct and then the second one the value is true.
After the initial event, all the next one are correct.

Please tell us about your environment:

  • Node-RED version: v1.2.9
  • Node.js version: v15.3.0
  • npm version: 7.0.14
  • Platform/OS: Windows 10
  • Browser: Google chrome in privacy mode
@knolleary
Copy link
Member

Yes, there's definitely something wrong here - both in the code and the docs.

The change event handler function should take three arguments, not the two suggested by the docs.

$(".input").on('change', function(event, type, value) {} );

But I can also see we were triggering the event incorrectly so the value argument wasn't getting passed through.

I've fixed that in the code - and will get the docs updated as well.

As for the 2 events you mention in the side note - I can't immediately see where that initial event (with true) is getting fired from. Will need to dig some more. What is odd is I can see that event getting triggered with the change event handler registered after the typedInput() call as well... investigation continues.

@Zehir
Copy link
Author

Zehir commented Feb 25, 2021

I tried to set a third argument in my callback but last was alway undefined. Maybe I test only with the type that don't have values.

@knolleary
Copy link
Member

I have tracked it down.

Once the edit dialog has been built, it automatically triggers the change event on all of the inputs that match node property names. As the standard input change event doesn't have any arguments, the editor passes a true as an internal flag to another piece of internal editor code.

We can probably find a different way of passing that flag so it doesn't show up to 'normal' event listeners, however it will mean the TypedInput change handler is invoked with undefined for both of its arguments as the editor code doesn't know the TypedInput has overloaded the Change event to provide those extra arguments.

Another option would be to make this editor code aware of the TypedInput behaviour and get it to do the right thing. I've tried to avoid doing that as it means the behaviour is now coded in two different places and adds an implicit dependency.

@Zehir
Copy link
Author

Zehir commented Feb 25, 2021

My javascript debugger say it's from this :

$("#"+prefix+"-"+d).trigger("change",[true]);

@knolleary
Copy link
Member

Yes - that's the code I was talking about in my previous comment

@Zehir
Copy link
Author

Zehir commented Feb 25, 2021

Yes - that's the code I was talking about in my previous comment

Yes I saw that after ;)

@knolleary
Copy link
Member

Fixed pushed to dev branch for the 1.3 release.

@Zehir
Copy link
Author

Zehir commented Feb 25, 2021

Thanks

@Zehir Zehir closed this as completed Feb 25, 2021
@Zehir Zehir reopened this Feb 26, 2021
@Zehir
Copy link
Author

Zehir commented Feb 26, 2021

@knolleary , I have the same issue when I register in oneditprepare for change event on the configuration node. Does he use typeinput internally and already fixed or it's an other issue ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants