Skip to content
This repository has been archived by the owner on Jan 3, 2020. It is now read-only.

sort out valueAsOption vs valueAsId in select controls (Combo, GridPicker etc) #7

Closed
dustingetz opened this issue Apr 1, 2014 · 4 comments
Assignees
Milestone

Comments

@dustingetz
Copy link
Contributor

Here is an email thread where Andrew and I brainstorm possible solutions. Just cleaning my inbox.

Andrew Goodale (Wingspan)
Feb 24

to me
I think that would be more deterministic, although I'm not sure what you mean by "manage the datasource by hand".
I know that kendo's combo tries to be smart, making secret requests for data when the value is set. I'm not a fan of magic like that, but I understand we need to work within what they do. Maybe the best choice, for "_Kendo_ComboBox", is to always pass the ID and let Kendo do its thing.

On Mon, Feb 24, 2014 at 9:43 AM, Dustin Getz (Wingspan) wrote:
so in both cases, we're going to bypass kendoCombo.value() and use kendoCombo.text() directly, and manage the datasource by hand?

On Mon, Feb 24, 2014 at 9:41 AM, Andrew Goodale (Wingspan) wrote:
You're not going to be able to have a library that dictates the server's data design choices - at least I don't think.
I guess what I'm trying to say is that the ComboBox would be happier if it didn't have to worry about this. Can we just externalize the whole issue, such that the ComboBox:

  1. Always treats "value" as an opaque thing.
  2. When it needs to render text for the value, it calls a function that's passed in. So the logic to determine how to display values is managed by the owner of the combo. Thus, whether the data is flat, joined, or whatever, the combo doesn't care.

On Mon, Feb 24, 2014 at 9:24 AM, Dustin Getz (Wingspan) wrote:
I am not sure how that would look. Can you elaborate a little more?

To recap the valueAs stuff:
if its always valueAsId, the object shapes are consistent (nothing joined) and always result in a datastore fetch. Thus all the code goes through the same "happy path" and there is more opportunity for abstraction. valueAsOption is the inline optimization which costs opportunities for abstraction because the business logic becomes coupled to the shape of the data, what is joined in and to which depth. So you couldn't build an AutoCrudApp for example.

On Fri, Feb 21, 2014 at 8:32 AM, Andrew Goodale wrote:
It's a little unusual. An alternative would be to allow an optional "valueRenderer" function, which would allow folks to handle any type of value. I can't remember all the reasons why a combo cares about the ID vs. Object issue, but it seems like it keeps the combo simpler to allow it to treat the value as opaque and let folks customize the rendering of the value.

If I was building a combo box from scratch, I would have "value" and "valueRender" props instead of "value", "idField" and "displayField".

AHG

On 20 Feb 2014, at 22:09, Dustin Getz (Wingspan) wrote:

I'm going to make the mode an explicit prop like mode={Modes.ValueAsId}.

If no mode is passed, it can default to "figure it out". I also am going to
add a console.warn every time this happens.

Can you think of any reason not to do this?

@newyankeecodeshop
Copy link
Contributor

I've been thinking more about this, and I think that you're right that we can make everything "valueAsId" to make the programming model simpler. So the "value" prop would take one or more IDs, and the "onChange" would return one or more IDs. My only addition to this is to have "onChange" also return the valueAsObject in a second argument. So that one could have:

onChange: function (value, valueObject) {
    var newRecord = _.clone(this.state.record);
    newRecord['fooID'] = value;
    newRecord['fooName'] = valueObject.name;
    this.setState({ record: newRecord });
}

I like using a second argument for the "valueAsObject" because then people who just want to stick with IDs can totally ignore it.
What do you think?

@newyankeecodeshop newyankeecodeshop self-assigned this Dec 26, 2014
@newyankeecodeshop newyankeecodeshop added this to the Release 1.0 milestone Dec 26, 2014
@dustingetz
Copy link
Contributor Author

Your proposal would lower the implementation complexity of valueAsObject and I think it's a good idea. The open question is if valueAsObject ought to be permitted at all as it limits how far you can abstract.

@newyankeecodeshop
Copy link
Contributor

At this point, I'd say to not permit "valueAsObject" at all. The client (or even AutoControl) can dereference object values pretty easily. My thought was to make AutoControl do this, so records that use object-style properties would be able to continue to work unchanged. AutoControl already knows what the "valueField" and "displayField" is for a combo...

newyankeecodeshop pushed a commit that referenced this issue Dec 29, 2014
AutoField/AutoControl now transfer all props to the underlying control.
KendoMultiSelect assumes ID values now. #7
@dustingetz
Copy link
Contributor Author

I agree with you and think permitting only valueAsId would result in simpler application code. Just note that "dereferencing an object value" in the most general way possible probably requires ajax, which can lead to a very request heavy app, which is why no consensus on this issue was reached. Though I would expect a lot of the requests can be mitigated through browser cache and hand optimization in hot spots. I also expect that this is a pretty involved refactor (for the TMF).

newyankeecodeshop pushed a commit that referenced this issue Jan 21, 2015
KendoComboBox onChange returns value ID and object - Part of #7
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants