UX: Add simple-list setting type (PR #9970)

don’t this this const is needed, I would just directly use this.values

once you change to @action you can do this.addValue(this.newValue)

please use @action

      return values.split(delimiter).filter(Boolean);

indentation of both is wrong here

indentation is wrong

why 0px and not 0 ?

can we use a class please?

can we use a class please?

Hmm, I was forced to do this by pre-commit hooks though, before I had this indentation, there were linting issues:


I copied the styles from the value-list SCSS, but found that this was an IE11 fix:


I know we don’t support IE11 anymore, but I would keep this as 0px for consistency.

correct indentation is:

  focus-out=(action "changeValue" index)


guess it would be cleaner to stop the event here if we do something with it

I’m unsure this has value as a fn here given we only use it once

I’m unsure this has value as a fn here given we only use it once

and same thing for _addValue

I would rather want to have an onChange event, instead of mutating this inside the component, values should be read only inside the component, given it’s coming from outside the component.

AFAIK keyCode is getting deprecated, I know we use it the app, but well if we are writing new code, maybe we should avoid it.


This is something slightly concerning that you are mixing KVO stuff (removeObject) and non KVO stuff (replace) it all works because in the end you do a set I guess.

Maybe you should consider calling arrayContentDidChange when doing replace, this way it will be consistent: https://api.emberjs.com/ember/3.18/classes/MutableArray/methods/removeObject?anchor=removeObject