microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.53k stars 2.73k forks source link

Dropdown.selectedKeys doesn't work if array is mobx @observable #10870

Closed ptallett closed 5 years ago

ptallett commented 5 years ago

Environment Information

Please provide a reproduction of the bug in a codepen:

If the selectedKeys array is an @observable from mobx, then Dropdown doesn't allow selection.

@observable selectedKeys: string[];

  <Dropdown placeholder="(All)" label="Plants" selectedKeys={this.selectedKeys} onChange={(e, item)=>this.onPlantChange(e, item)}
              multiSelect options={this.options} styles={{ dropdown: { width: 257 } }}/>

Actual behavior:

Unable to select items in the dropdown

Expected behavior:

Dropdown should update when items are clicked.

Priorities and help requested:

Are you willing to submit a PR to fix? (Yes, No) No

Requested priority: (Blocking, High, Normal, Low) Normal

Products/sites affected: (if applicable) Simple workaround - this.selectedKeys.slice()

cliffkoh commented 5 years ago

HI @ptallett, the Dropdown selectedKeys prop expect a very specific type and the @observable decorator has likely violated the "contract". We are not able to have all our components support MobX decorated types because our component library is meant to be data-flow agnostic and there are real costs (in bundle size) to adding framework specific support. We are therefore very unlikely to ever "fix" this issue. (Fortunately, as you have also identified, there are easy workarounds).

ptallett commented 5 years ago

You could trivially put the slice in the codebase.

cliffkoh commented 5 years ago

Unfortunately, we need to be principled about things like this. A component library needs some amount of "coherence" in its API surface and it would simply not work for only one random prop to apply slice() while all our other props do not. There are also other considerations as well but hopefully this helps you understand a bit why we must decline adding slice into the component layer at this point.

msft-github-bot commented 5 years ago

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!