feat(ux-select): ux-select element

[bigopon]

(I captured the gif at 15fps so it didn't show hide animation 😓 )

Usage is similar to native combo <select/> / <option/>:

<ux-select
    value.bind='selected'
    multiple.bind='uxSelectMultiple'>
  <ux-option>Testing</ux-option>
  <ux-optgroup disabled label='Group 1'>
    <ux-option>Testing 2 asd asdas asd sad asdsad asds</ux-option>
  </ux-optgroup>
  <ux-optgroup label='Group 22'>
    <ux-option>Testing 1 2 3</ux-option>
  </ux-optgroup>
</ux-select>

Because the way replaceable /part works is different with <slot/>, I think we will provide another implementation like <ux-dropdown/> or <ux-combobox/>, where an item source is required to be supplied, with optional item template, similar to my other PR of <ux-select/> at #132

@EisenbergEffect @ZHollingshead It's a bit of a pain to setup gist run so probably please pull it down to review 😄

Note: it requires <ux-checkbox/> to be used as well

[bigopon]

@ZHollingshead @EisenbergEffect

Design spec: from material design(-ish)
Tech spec: from native implementation(-ish)

Difference compared to native:

• <ux-option/> value can be anything, and value is kept as is, there is no need for a model property to handle non string value:

  <!-- native -->
  <select value.bind='selection'>
    <!-- cannot use value.bind='someObject' because it will be stringify -->
    <option model.bind='someObject'>Text</option>
  </select>
  
  <!-- ux -->
  <ux-select value.bind='selection'>
    <ux-option value.bind='someObject'>Text</ux-option>
  </ux-select>

• switching between multiple and single mode after initialization will reset value of the <ux-select/>:

  • single -> multiple: reset to empty array
  • multiple -> single: reset to null

[EisenbergEffect]

What does it mean to have an items property and a list of ux-option child nodes at the same time?

[bigopon]

items was there via copy paste from an example of old implementation, I edited the commit and removed it. This implementation is very close to native, except some difference mentioned above

[EisenbergEffect]

I may not get a chance to review until the weekend, but this looks very exciting. @bigopon Have you tested this with a repeat-for on the ux-option and/or the group? I want to make sure that these can be data-driven effectively.

[bigopon]

@ZHollingshead At the moment I'm using <ux-checkobx/> for the checkbox in multiple select mode. This has cons:

• <ux-checkbox/> needs to be loaded before <ux-select/>
• a maybe unnecessary dependency.

I think for the purpose of <ux-select/>, we only need a pseudo checkbox for visual effect. What do you think ?

[serifine]

@bigopon I think it is unnecessary if we are just showing a checked state. I am not sure I would use a checkbox at all to be honest, the border and then the colors of the checked content can make the select menu look a bit.. cluttered.

I like the idea of the items 'checking' though. You could try to just include the check mark SVG used in checkbox. That would display only a check mark next to a selected item which would give us the checked appearance, while also looking visually clean. What do you think?

Checkmark SVG:

<svg viewBox="0 0 24 24">
        <path d="M0 0h24v24H0z" fill="none" />
        <path d="M9 16.17L4.83 12l-1.42 1.41L9 19 21 7l-1.41-1.41z" />
</svg>

[bigopon]

Ill update the pr soon. :+1

[bigopon]

@ZHollingshead @EisenbergEffect updated the pr No longer requires ux - checkbox

[EisenbergEffect]

Cool stuff @bigopon Can you talk about the swizzling of the prototype for the elements? Was that mostly a convenience? or was there something else you had in mind there?

@ZHollingshead Can you review as well?

[bigopon]

@EisenbergEffect

One reason is to have synchronous behavior when notifying and receiving changes, which I believe is better when building a components lib like ux. Another reason is to have a unified change event for all inputable ux element to notify change. The original work was started here #141 . That was for the value / checked property on the element prototype, as observers from aurelia-binding work with those properties on the element.

Also for <ux-option/>, @children is insufficient, as it works with child nodes on the same level, while <ux-option/> can be child of either <ux-optgroup/> or <ux-select/> , and after that, I don't think we would want to eagerly resolve their underlying view models, as they are not often needed straightaway, so I return the <ux-option/> element instead, and depends on the accessed property, will resolve to different property on the view model.

[serifine]

I am unable to build this by simply pulling down the repo and doing the following
npm install
lerna bootstrap --hoist
lerna run build

Here are some of the errors I got:

Core Package

In extension.ts the following error appears:

[4] src/extension.ts(2,3): error TS6133: 'InternalPropertyObserver' is declared but its value is never read.

In extension.ts the following linting issues appear:

src/extension.ts[9, 20]: An interface declaring no members is equivalent to its supertype.
src/extension.ts[11, 20]: An interface declaring no members is equivalent to its supertype.

This probably just needs the '// tslint:disable-next-line:no-empty-interface' line copied before each method.

Checkbox Package

[4] src/index.ts(1,74): error TS6133: 'CheckedObserver' is declared but its value is never read.
[4] src/index.ts(19,20): error TS2693: 'CheckedObserver' only refers to a type, but is being used as a value here.
[4] src/ux-checkbox.ts(155,43): error TS2345: Argument of type 'Window' is not assignable to parameter of type 'Element'.

[serifine]

@bigopon

This was on a Windows 10 Machine by the way.

NPM Version: 5.6.0
Node JS Version: v9.5.0

[serifine]

One other thing I just noticed. The folder should probably be called select rather than ux-select to match with the other components

[serifine]

select is also missing from the lerna.json file.

[serifine]

@bigopon everything looks good, is this good to merge?