dart-archive / polymer-dart

Polymer support for Dart
https://pub.dartlang.org/packages/polymer
BSD 3-Clause "New" or "Revised" License
180 stars 33 forks source link

paper-menu, select method adds extra value to selectedValues #691

Open jonboj opened 8 years ago

jonboj commented 8 years ago
Environment/Code

A paper-menu with multi selection. Dartium html

<paper-button on-tap="deSelectAll">Deselect All</paper-button>
<paper-menu id="id_pm" multi selected-values="{{selectedValues}}">
      <paper-item>A0</paper-item>
      <paper-item>A1</paper-item>
</paper-menu>

dart

...
@property
List<String> selectedValues = [];
...
PaperMenu pm = $$('#id_pm') as PaperMenu;
pm.select('0');
...
What is expected

For a value in selectedValues should select insert and remove the value from selectedValues.

Actual outcome

First time select(v) is called an extra instance of v is added into the selectedValues. This instance remains when the item is deselected with tap from Dartium.

Howto reproduce

console print dumps.

SelectTest.onIronSelect, selected : [0]
SelectTest.onIronSelect, selected : []
SelectTest.onIronSelect, selected : [0]
SelectTest.deSelectAll
SelectTest.deSelectAll, [0]
SelectTest.deSelectAll, select('0')
SelectTest.onIronSelect, selected : [0, 0]
SelectTest.onIronSelect, selected : [0]
SelectTest.onIronSelect, selected : [0, 0]

Cut from pub.yaml

environment:
  sdk: '>=1.9.0 <2.0.0'

dependencies:
  polymer: ^1.0.0-rc.15
  web_components: ^0.12.0
  polymer_elements: 1.0.0-rc.8
  browser: ^0.10.0
  reflectable: ^0.5.0
  polymer_interop: ^1.0.0-rc.5
zoechi commented 8 years ago

Sounds similar to dart-lang/polymer_elements#92 Might be an issue with array-selector. I guess this is used for selection of such lists (not sure).

jonboj commented 8 years ago

Interesting, this gave me a clue. Debugging into iron-multi-selectable.html

The tap selection from Dartium puts an int 0 into this.selectedValues, but the select method from Dart comes with a String "0" so at this line https://github.com/dart-lang/polymer_elements/blob/master/lib/src/iron-selector/iron-multi-selectable.html#L62 it don't matches and pushes "0" into the array, this gives the two instance. Same index, but different type reprensentations int/String.

------------------------------- Update -------------------------------- With some debugging in the call stacks for the two cases.

In Dartium tap on menu item, the int case.

The tap event from Dartium gets converted to an int value at this location https://github.com/dart-lang/polymer_elements/blob/master/lib/src/iron-selector/iron-selectable.html#L308 For later down the call stack end as value to a select call.

From dart code when PaperMenu.select is called, the String case. At api level https://github.com/dart-lang/polymer_elements/blob/master/lib/iron_multi_selectable.dart#L36

------------------------- _indexToValue ------------------------- Looking at the method _indexToValue if attrForSelected isn't set, it just return the index. https://github.com/dart-lang/polymer_elements/blob/master/lib/src/iron-selector/iron-selectable.html#L255

If attrForSelected isn't set value is item, so in _indexToValue it seems more logic if returned this.items[index];here: https://github.com/dart-lang/polymer_elements/blob/master/lib/src/iron-selector/iron-selectable.html#L261

jonboj commented 8 years ago

Reproduced with polymer_elements: ^1.0.0-rc.9

From pubspec.yaml

environment:
  sdk: '>=1.9.0 <2.0.0'

dependencies:
  browser: ^0.10.0
  polymer_elements: ^1.0.0-rc.9
  polymer: ^1.0.0-rc.17
  web_components: '>=0.11.3 <0.13.0'