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

Default selected options are empty #147

Closed
VMBindraban opened this issue Aug 23, 2016 · 20 comments
Closed

Default selected options are empty #147

VMBindraban opened this issue Aug 23, 2016 · 20 comments
Assignees

Comments

@VMBindraban
Copy link

I'm submitting a bug report

  • Library Version:
    1.0.0

Please tell us about your environment:

  • Operating System:
    Linux Ubuntu 16.04
  • Node Version:
    4.4.4
  • NPM Version:
    2.15.1
  • JSPM OR Webpack AND Version
    JSPM 0.16.36
  • Browser:
    Chrome 51
  • Language:
    all

Current behavior:
When using a translation on the first option, no fields are default selected.

Expected/desired behavior:

Gist.run is down.

<template>
  <select class="form-control" value.bind="value" multiple.bind="multiple">
    <option t="- Select a value -" value="0">- Select a value -</option>
    <option model.bind="option.id" repeat.for="option of options">${option[property]}</option>
  </select>
</template>

Value contains an array with the selected items.
When using translation (see above), the value becomes null between the bind and attached cycle. When removing t="- Select a value -", it works.

  • What is the expected behavior?
    Given values should be selected.
  • What is the motivation / use case for changing the behavior?
    It doesn't work correctly.
@zewa666
Copy link
Member

zewa666 commented Aug 23, 2016

Gist.run is up and running again, please provide an example.

@VMBindraban
Copy link
Author

https://gist.run/embed.html?id=761ac31e78d1110224f0e61e32db3e27

I couldn't import the i18n libary however. Without that the bug will not appear.

@zewa666
Copy link
Member

zewa666 commented Aug 23, 2016

I've replicated it locally and both values are selected in both examples. So it seams something else is causing the issue maybe? One thing I noticed is your translation key Select something. Is that really the key, containing a blank space? Btw. I've tried it with the recent release

@VMBindraban
Copy link
Author

VMBindraban commented Aug 23, 2016

I will look further then. I never had issues with keys containing spaces.

@VMBindraban
Copy link
Author

VMBindraban commented Aug 24, 2016

It seems that when i add [html] to the t parameter, it works.

I can't find any leads that it's on my side besides that in valueChanged() the value gets changed into an empty array with a ModifyArrayObserver. Is it possible to trace the observer?

I can't make a working demo but i am sure that i18n is causing this.

@VMBindraban VMBindraban reopened this Aug 24, 2016
@zewa666
Copy link
Member

zewa666 commented Aug 24, 2016

ok so can you create a zip file containing the CLI src folder, aurelia.json and locales and upload it somewhere, so that I can reproduce it?

@doktordirk
Copy link
Contributor

@VMBindraban if you wanna use cli, you can use the swan-client-sample aurelia-cli branch

@VMBindraban
Copy link
Author

VMBindraban commented Aug 24, 2016

I managed to isolate the issue in git.run. You need to import aurelia-i18n (again).

https://gist.run/embed.html?id=761ac31e78d1110224f0e61e32db3e27

When you look in the console i am logging the value in valueChanged().
The first change is to id 10 (which is good), the 2e change is into Foo. (which is the translated key, not good).

What i assume that happens, when i18n is changing the value, the select sees that as a change and sets the value to the translated option. Is this a possible binding issue?

@zewa666
Copy link
Member

zewa666 commented Aug 24, 2016

Ok so after our conversation in gitter @VMBindraban I think we've traced the bug a little bit further.

If we console log the valueChanged with new/old value we get 3 calls:

null -> 10
10 -> foo
foo -> translation

call 1 is the default setting it from empty to the id.
call 2 is caused by this line https://github.com/aurelia/i18n/blob/master/src/i18n.js#L159
call 3 because of whatever.

@jdanyow do you have an idea why the promised call to updateValue, caused from the TCustomAttribute interfers with the Changed hook in the component?
But even after switching there to sync, why is the 3rd call existing?

@RWOverdijk
Copy link

👍 Support + watching thread.

@VMBindraban
Copy link
Author

Bump. Any progress on this?

@jdanyow
Copy link

jdanyow commented Sep 12, 2016

I'll take a look

@VMBindraban
Copy link
Author

@jdanyow Any news on this?

@VMBindraban
Copy link
Author

@zewa666 Bump, any progress? It's kinda delaying our project.

@zewa666
Copy link
Member

zewa666 commented Oct 7, 2016

Sorry about the delay, @jdanyow is pretty busy with getting things done for the Validation Plugin and referenced work in other framework parts. In the meantime can you @VMBindraban tell me whether changing the promised return to a straight forward one inside the updateValue method fixes your issue?

updateValue(node, value, params) {
    if (params) {
      this.params[value] = params;
    } else if (this.params[value]) {
      params = this.params[value];
    }

    //return this.i18nextDefered.promise.then(() => this._updateValue(node, value, params));
    return this._updateValue(node, value, params);
  }

the easiest way to test it out is if you're using Aurelia CLI, to open up the app-bundle.js and search for the transpiled version

I18N.prototype.updateValue = function updateValue(node, value, params) {

in there just fix the above mentioned line.


It feels like this deferred call is already responsible for multiple issues, so by having your positive answer to that I guess I'll simply go ahead and publish a fix release. We can determine the root cause for this later, without blocking you guys any further.

@zewa666 zewa666 self-assigned this Oct 7, 2016
@jdanyow
Copy link

jdanyow commented Oct 7, 2016

@VMBindraban I took a look at the gist- couple question for you:

  1. In association-select.html, there's a repeat.for="option of options"... where is options defined: https://gist.github.com/VMBindraban/761ac31e78d1110224f0e61e32db3e27#file-association-select-html-L4
  2. In association-select.html, there's an option with no model. Shouldn't this element have model.bind="null" or model.bind="0": https://gist.github.com/VMBindraban/761ac31e78d1110224f0e61e32db3e27#file-association-select-html-L3

zewa666 added a commit that referenced this issue Oct 7, 2016
params are reissued properly on locale change. Removal of unnecessary
Promise wrapping of updateValue.

Fixes issue
#156
#147
#143
#116
@zewa666
Copy link
Member

zewa666 commented Oct 7, 2016

@jdanyow @VMBindraban ok I think I have now a better understanding of this Promise thingy ... it was introduced with a previous enhancement which turned out to introduce a lot of regressions. There is a new PR pending. As for this issue if @VMBindraban you could just verify that the change I proposed previously I can merge the PR right away and get 4 bugs done straight in a row :)

@VMBindraban
Copy link
Author

@zewa666 That didn't work.
params is always undefined, value is - Select a value -

@jdanyow I only kept de bare minimal to duplicate the issue:
The actual code can be found here:
https://github.com/SpoonX/aurelia-orm/blob/master/src/component/association-select.html
https://github.com/SpoonX/aurelia-orm/blob/master/src/component/association-select.js

options is an array containing objects. An object contains the following:

{
  id:1,
  name: "foo"
  createdAt:"2016-10-07T12:57:44.000Z",
  updatedAt:"2016-10-07T13:11:50.000Z"
}

I tried both model.bind="null" and model.bind="0" with no success.

zewa666 added a commit that referenced this issue Oct 11, 2016
params are reissued properly on locale change. Removal of unnecessary
Promise wrapping of updateValue.

Fixes issue
#156
#147
#143
#116
@zewa666
Copy link
Member

zewa666 commented Oct 11, 2016

I've merged the PR anyway since its part of this bug as well. It definitely won't harm, but we need to closer look at the current issue. @jdanyow any new insights?

@VMBindraban
Copy link
Author

The PR did not resolve the problem. For now we are going to use t="[html]string" as temporary fix.

mohammedJ pushed a commit to mohammedJ/i18n that referenced this issue Apr 6, 2017
and `npm run e2e` script
references aurelia#147
@VMBindraban VMBindraban closed this as not planned Won't fix, can't repro, duplicate, stale Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants