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

Dynamic filled select show wrong t-model value #1444

Closed
nseinlet opened this issue May 26, 2023 · 4 comments · Fixed by #1469
Closed

Dynamic filled select show wrong t-model value #1444

nseinlet opened this issue May 26, 2023 · 4 comments · Fixed by #1469
Labels
bug Something isn't working enhancement New feature or request

Comments

@nseinlet
Copy link

A component with a input type='select' for which the values comes dynamically didn't show the correct value.
In this example, We have 2 selects, one for an element, and one for the element value.

the second select is filled with options filtered based on the first select. the t-model store the correct value, but didn't print the correct value when choosing an element in the fist select.

image

const { Component, mount, xml, useState } = owl;

class EditElement extends Component {
    static props = [ 'vals', 'edit_elem' ];
    static template = xml/* xml */ `
    <div>
        <t t-raw="props.edit_elem.num"/>

        <select name="elem_val" class="form-select" t-model="props.edit_elem.num">
            <t t-foreach="props.vals.choices[props.edit_elem.name]" t-as="it" t-key="it.val">
                <option t-esc="it.val" t-att-value="it.val"/>
            </t>
        </select>
    </div>
    `;
    setup() {
    };
}

class Root extends Component {
    static components = { EditElement };
    static template = xml/* xml */ `
      <div>
        Choose:
        <select name="section" class="form-select" id="sel_item" t-model="vals.selected">
            <t t-foreach="vals.elems" t-as="it" t-key="it.name">
                <option t-esc="it.name" t-att-value="it.name"/>
            </t>
        </select>
      </div>
      <div>
        Then edit:
        <t t-if="vals.selected">
            <EditElement vals="vals" edit_elem="vals.elems.filter(e => e.name==vals.selected)[0]"/>
        </t>
      </div>
      `;


    setup() {
        this.vals = useState({
            selected: "A",
            elems: [
                {name: "A", num: 1},
                {name: "B", num: 4},
            ],
            choices: {
                A: [{val: 1}, {val: 2}, {val: 3}],
                B: [{val: 4}, {val: 5}, {val: 6}],
            }
        });
    };
  }

  mount(Root, document.body, {dev: true});

The t-raw in the EditElement always show the correct value, while the input type='select' always show the first value.

@sdegueldre sdegueldre added the enhancement New feature or request label May 30, 2023
@sdegueldre
Copy link
Contributor

You can work around this issue for now by using t-att-selected="it.val == props.edit_elem.num" on the <option>, but I'll have to agree that this should really already be handled by t-model

@sdegueldre sdegueldre added the bug Something isn't working label May 30, 2023
ged-odoo added a commit that referenced this issue Jun 29, 2023
Before this commit, owl parser would not allow applying the `.number`
suffix on <select> options. I do not see a good reason for that, and it
prevents some legitimate usecases.

closes #1444
@ged-odoo
Copy link
Contributor

As far as I can tell, if we use the option t-att-value attribute, we basically serialize the value to a string, and we need then to retransform it to a number when we apply the change with the t-model directive.

So, I think that the example is wrong: it should use t-model.number in the EditElement template. But if you try that, you will notice that Owl ignore it anyway, which is a bug, fixed by pr #1469 .

@ged-odoo
Copy link
Contributor

Note that we probably could modify owl store each value in a map somewhere. Then, when an option is selected, it could look that value in the map (indexed by the stringification of the value) and give it back in the t-model expression. That would probably work exactly as expected. With this, there is no need for a .number suffix on `t-model.

But then, it may be slightly surprising, because one might expect the value to be a string, since it seems to be an attribute. Probably not, but who knows? An opinion on this, @sdegueldre ?

@sdegueldre
Copy link
Contributor

I'm not sure I understood the question 😅

ged-odoo added a commit that referenced this issue Jul 12, 2023
Before this commit, owl parser would ignore the `.number` suffix on
<select> options. I do not see a good reason for that, and it prevents
some legitimate usecases.

closes #1444
sdegueldre pushed a commit that referenced this issue Jul 12, 2023
Before this commit, owl parser would ignore the `.number` suffix on
<select> options. I do not see a good reason for that, and it prevents
some legitimate usecases.

closes #1444
phil-form pushed a commit to phil-form/owl that referenced this issue Nov 7, 2023
Before this commit, owl parser would ignore the `.number` suffix on
<select> options. I do not see a good reason for that, and it prevents
some legitimate usecases.

closes odoo#1444
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants