Skip to content
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

Add SelectWidget support for grouped options and placeholder prop #8468

Merged
merged 10 commits into from
May 17, 2024

Conversation

mnholtz
Copy link
Collaborator

@mnholtz mnholtz commented May 16, 2024

What does this PR do?

  • Part of https://github.com/pixiebrix/pixiebrix-app/issues/5209
  • I need SelectWidget to be able to handle grouped options (it currently makes the assumption that the options prop isn't grouped, and thus passing grouped options breaks the selected option behavior)
  • Also adding the placeholder prop while I'm add it, which we'll also need

Discussion

  • TODO: Fix typing; the generics are going to need to be tweaked here

Demo

This change enables grouped options and custom placeholder text, e.g.:
Screenshot 2024-05-16 at 8 46 10 AM

Checklist

  • Add jest or playwright tests and/or storybook stories
  • Designate a primary reviewer @grahamlangford

@mnholtz mnholtz requested review from grahamlangford and BLoe May 16, 2024 15:47
@mnholtz mnholtz marked this pull request as draft May 16, 2024 15:48
Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.43%. Comparing base (6d95047) to head (057670a).
Report is 71 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8468      +/-   ##
==========================================
- Coverage   73.47%   73.43%   -0.04%     
==========================================
  Files        1334     1352      +18     
  Lines       41259    41562     +303     
  Branches     7686     7799     +113     
==========================================
+ Hits        30316    30523     +207     
- Misses      10943    11039      +96     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented May 16, 2024

Playwright test results - MV2

passed  44 passed
skipped  12 skipped

Details

report  Open report ↗︎
stats  56 tests across 19 suites
duration  11 minutes, 57 seconds
commit  057670a

Skipped tests

chrome › tests/extensionConsoleActivation.spec.ts › can activate a mod with built-in integration
chrome › tests/extensionConsoleActivation.spec.ts › can activate a mod via url
edge › tests/extensionConsoleActivation.spec.ts › can activate a mod with built-in integration
edge › tests/extensionConsoleActivation.spec.ts › can activate a mod via url
chrome › tests/regressions/sidebarLinks.spec.ts › #8206: clicking links from the sidebar doesn't crash browser
edge › tests/regressions/sidebarLinks.spec.ts › #8206: clicking links from the sidebar doesn't crash browser
chrome › tests/runtime/sidebarController.spec.ts › sidebar controller › shows focus dialog in top-level frame
edge › tests/runtime/sidebarController.spec.ts › sidebar controller › shows focus dialog in top-level frame
chrome › tests/runtime/sidebarNavigation.spec.ts › sidebar mod panels are persistent during navigation
chrome › tests/runtime/sidebarNavigation.spec.ts › sidebar form and temporary panels are unavailable after navigation
edge › tests/runtime/sidebarNavigation.spec.ts › sidebar mod panels are persistent during navigation
edge › tests/runtime/sidebarNavigation.spec.ts › sidebar form and temporary panels are unavailable after navigation

Copy link

github-actions bot commented May 16, 2024

Playwright test results - MV3

passed  56 passed

Details

report  Open report ↗︎
stats  56 tests across 19 suites
duration  16 minutes, 44 seconds
commit  057670a

@mnholtz mnholtz marked this pull request as ready for review May 16, 2024 23:05
Copy link

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

@@ -29,12 +29,18 @@ import useAddCreatablePlaceholder from "@/components/form/widgets/useAddCreatabl
// Type of the Select options
export type Option<TValue = string> = {
label: string;
value: TValue | null;
value: TValue;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for TValue | null, since we can create type NullableOption = Option<string | null> and it just works

@twschiller twschiller added user experience Improve the user experience (UX) enhancement New feature or request labels May 17, 2024
@mnholtz mnholtz merged commit 0fd35f4 into main May 17, 2024
19 checks passed
@mnholtz mnholtz deleted the feature/support_select_widget_grouped_options branch May 17, 2024 16:51
@twschiller twschiller added this to the 2.0.0 milestone May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request user experience Improve the user experience (UX)
Development

Successfully merging this pull request may close these issues.

4 participants