observablehq / inputs

Better input elements
https://observablehq.com/framework/lib/inputs
ISC License
128 stars 34 forks source link

Inputs.select({keyof: <function>}) documentation doesn't match behavior: keyof doesn't affect text displayed in selection list #211

Closed curiouserrandy closed 2 years ago

curiouserrandy commented 2 years ago

https://github.com/observablehq/inputs/blob/main/README.md#Select documents the function of the "keyof" option to Inputs.select(): "The elements in data need not be strings; they can be anything. To customize display, optional keyof and valueof functions may be given; the result of the keyof function for each element in data is displayed to the user....". However, it does not appear that the keyof specification changes what is displayed to the user. In https://observablehq.com/@curiouserrandy/demo-of-believed-bug-in-inputs-select I've created a small example that tries to use keyof to pull out the id of a series of objects in an array, but Inputs.select() still displays "[object Object]" for each element of the array.

Examining the source in https://github.com/observablehq/inputs/blob/86fa5c0b529c32cd83fccdf9b6b80ffe3bf015c0/src/chooser.js#L10 (linked from the documentation indirectly) it appears that the keyof function is only used to create a list of keys, and that list is only used to uniquify the array if unique is specified. But I don't guarantee I'm reading the source correctly.

mbostock commented 2 years ago

Currently you need to specify the format option for this, not the keyof option:

viewof example_select = Inputs.select(object_list, {
  format: (d) => d.id,
  valueof: (d) => d.data,
  multiple: true
})

Despite what the documentation says, the keys aren’t currently actually displayed to the user. They are used (1) to remove duplicate options (with non-unique keys) in conjunction with the unique option, and (2) to sort the options in conjunction with the sort option.

I think we can fix this by changing the default format option to compose with the given keyof function, if any. That way if you specify the keyof option, it will implicitly affect the format, too.

(There’s also the related bug #128 where the key option only works if data is a Map, but it should probably work if you specify a keyof function, too.)

curiouserrandy commented 2 years ago

Ah, thank you! We can switch this bug over to "bring the documentation and code into alignment", I don't think it matters which way. I'll edit the subject line.

tophtucker commented 1 year ago

Thanks for reporting this forever ago, @curiouserrandy. We totally neglected upgrading the actual deployed version of Inputs for way too long, but Mike's fix is now finally live on the site. We've improved our internal processes for upgrading these libraries so more people can do it (like… me!) and it'll be less of a bottleneck in the future.

Test cases for the new release: https://observablehq.com/d/4303b3734fb950d8