-
-
Notifications
You must be signed in to change notification settings - Fork 505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed lagging state of multiselect FluentUI component and added searchable options to select and multiselect #1090
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 09a6f3f:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1090 +/- ##
==========================================
- Coverage 82.15% 82.10% -0.06%
==========================================
Files 211 212 +1
Lines 11099 11107 +8
Branches 1332 1335 +3
==========================================
+ Hits 9118 9119 +1
- Misses 1367 1373 +6
- Partials 614 615 +1 ☔ View full report in Codecov by Sentry. |
Thanks for the PR. Search in select boxes looks cool. However I see one issue with the line you've deleted selectedKeys={selectedKeys} Multiselect ignores
|
Btw, I think that using react-awesome-query-builder/packages/fluent/modules/widgets/value/FluentUIMultiSelect.jsx Line 10 in 7057376
https://github.com/ukrbublik/react-awesome-query-builder/blob/master/packages/examples/demo/config.tsx#L572 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove selectedKeys
state from FluentUIMultiSelect
component and pass selectedKeys
prop to Dropdown
I think my last commit should deal with the useless state, and passed the value directly to the dropdown |
Forgot to run the linter.... |
I ran into an issue where when I used the fluent-ui multiselect the state of the selection would lag behind the selection in the component itself. This happened because there where two react state setters where the second setter depended on the value set by the first:
My fix was to determine the selection separate from the setters and pass that to both setters in one go:
Then I also lacked search in the fluent-ui select & multiselect widgets, I contemplated adding it by switching to fluent's ComboBox component since it seemed a bit more in-line with how fluent prescribes the usecase. But that way it didn't really match the way the other ui-framework widgets do search, which is admittedly a bit nicer than the combobox.
Ultimately I made a 'SearchableDropdown' component based on a reply on this Fluent-UI issue and used that instead.