Investigate an observable internalOptions prop on hoist dropdown fields

[febbraiod]

When dynamically changing the option underneath one these fields we expect to see the options rerender in the field's menu. This seems to work as expected when using the @observable.ref decorator on set of options we pass in as props, and then reassigning that array to change it's value. However, if we use just @observable and then push to that array we are seeing the outer component that the field is nested in rerender, but not the internal field component itself.

While using the ref for our props is perfectly acceptable and somewhat more clear, there is a concern that we are relying on some implementation detail in mobx, and cannot rely on this always being the case. We should therefore look to somehow rerender these fields with certainty.

[febbraiod]

At very least we need to declare the internalOptions prop on the basedropdown class

[amcclain]

Thanks for filing, Don!

I think it is important that the internalOptions property be observable to ensure that the component re-renders whenever those internal options are changed (as a result of the options passed via props changing, in any way).

Currently the render method of the parent (the container specifying the selectField) can run if those outer options are updated (i.e. pushed onto). but that alone doesn't trigger a re-run of the selectField's own render method. Unless the entire options collection has changed, the component hasn't received any new/different props - it's the same collection, and nothing deep is happening by default at the other level.

The autorun setup internally by selectField will run, updating internalOptions correctly, but without render running it won't reflect changes to those opts.

SelectField does not access its props.options object within its internal method, and even on the individual item level, normalizeOptions strips out the MobX proxy objects that power the "deep" change detection on the individual option (object) level. Only by making internalOptions observable do we get that change detection back on the data structure that's actually accessed within render().

It might still be a best practice to replace the entire options collection and/or avoid mutating the options directly on the individual object level. But I don't think we should require that...

[amcclain]

Believe the P/R above is the minimal incremental change, based off of the current develop branch. The change to call the setInternalOptions action method would need to be made in the multiSelect control we have in another active branch.

While making this change, I was wondering if we want this multi-level object hierarchy for dropdown fields. I think it's confusing things more than it might be helping, especially as QueryComboField doesn't follow the same patterns, and we have this awkward naming around dropdown vs. combo base classes (I can never get it in my head which is which). It would be a distinct ticket, for sure, but I would vote for looking at that change and seeing how much code duplication it would entail vs. allowing a flatter and simpler hierarchy.

[lbwexler]

I am fine with the idea of flattening the dropdown field hierarchy

[lbwexler]

And thanks for the analysis Anselm. Don -- just want to make sure its clear-- changing the options reference passed, actually changes the prop value itself, which is what was causing the re-render when you change the ref, according to the standard react conventions. The @observable.ref marking on the options passed in was actually doing nothing....

[febbraiod]

Ah yes that makes perfect sense. Sorry I missed that fact