From f88bc7e18aebd182bb68d8829b8c3cc5c63ded4d Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Tue, 23 Jun 2026 06:33:47 -0300 Subject: [PATCH] picker: Simplify presentation and sizing APIs, add preview-aware defaults (#59719) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recent "pickers with previews" overhaul left the picker sizing/presentation API spread across several overlapping knobs that every call site had to set correctly — and many didn't, causing pickers across the app to render at the wrong width, lose their container, or stop dismissing. This PR consolidates that surface into a small, hard-to-misuse API and makes correct sizing the default. Net effect: a plain `Picker::uniform_list(delegate, …)` now renders correctly out of the box (standard width, standard max-height, shrinks to fit, dismisses properly), and the ~35 call sites only specify what genuinely differs. ## API changes **Presentation** — three overlapping booleans (`is_modal`, `is_popover`, `is_resizable`) collapsed into one enum, with resizability living inside the only variant where it's meaningful: ```rust enum Presentation { Modal { resizable: bool }, // own chrome, dismisses on blur, optionally resizable Popover, // own chrome, dismisses on blur, never resizable Embedded, // host container owns chrome + dismissal } ``` - `modal(bool)` is **removed** in favor of explicit, self-documenting builders: - *(default)* → `Modal` (resizable iff it has a preview) - `.popover()` → `Popover` (menu-attached surfaces) - `.embedded()` → `Embedded` (pickers nested in a larger modal/view) - Dynamic callers use `.when(cond, Picker::embedded)` (added `impl FluentBuilder for Picker`). **Sizing** — preview-vs-not now drives everything; the manual padding knob is gone: | Before | After | |---|---| | `vertical_padding` field + `no_vertical_padding()` | removed — derived from whether a preview is visible | | `height(...)` (ambiguous: fixed vs max) | `max_height(...)` (plain pickers shrink-to-fit, capped here) | | `minimum_results_width(...)` | removed — a plain picker's min width tracks its opening width; preview pickers use standard internal pane mins | | default size = 60% viewport | default = `DEFAULT_MODAL_WIDTH` (34rem) × `DEFAULT_MODAL_MAX_HEIGHT` (24rem, max) | | resize handles gated on `is_modal` | gated on `is_resizable` (new `resizable(bool)` builder; auto-`true` for preview pickers) | Call sites now only override the exceptions: narrow popover selectors (`initial_width`), the taller outline view (`max_height`), and preview pickers (constructed via `*_with_preview`). ## Behavior fixes - **Wrong widths everywhere**: pickers were falling back to 60%-of-viewport because the original migration set `minimum_results_width` but never `initial_width`. Fixed at the source via the new defaults. - **Popovers had no container and wouldn't dismiss**: `is_modal=false` was suppressing both the elevated background *and* blur-dismiss. Split out so popovers keep their chrome and dismiss on click-away/escape. This fixed the agent-panel model/profile selectors, sidebar recent projects, and the settings theme/font/icon/ollama pickers (which were incorrectly using `modal(false)`). - **Sidebar recent projects stretched to full height**: was missing the shrink-to-fit behavior; now capped and content-sized like other popovers. - **Preview crash**: removed an over-strict `debug_assert!` that panicked when previewing an empty file (`message == None && editor empty` is valid). - **Preview-aware default size**: pickers open at standard width with the preview hidden, and expand to the larger "telescope" size when a preview is shown. Fixes the text finder rendering super-wide by default, and makes the file finder expand (rather than cram its results) when you toggle the preview. --- Release Notes: - N/A --- assets/keymaps/default-linux.json | 4 +- assets/keymaps/default-macos.json | 4 +- assets/keymaps/default-windows.json | 4 +- assets/keymaps/specific-overrides-macos.json | 2 +- assets/keymaps/specific-overrides.json | 2 +- .../manage_profiles_modal.rs | 2 +- .../src/agent_configuration/tool_picker.rs | 18 +- crates/agent_ui/src/config_options.rs | 4 +- .../agent_ui/src/language_model_selector.rs | 9 +- crates/agent_ui/src/model_selector.rs | 4 +- crates/agent_ui/src/profile_selector.rs | 4 +- crates/agent_ui/src/threads_archive_view.rs | 6 +- .../src/collab_panel/channel_modal.rs | 6 +- .../src/collab_panel/contact_finder.rs | 9 +- crates/command_palette/src/command_palette.rs | 6 +- crates/debugger_ui/src/attach_modal.rs | 6 +- crates/debugger_ui/src/new_process_modal.rs | 4 +- crates/dev_container/src/lib.rs | 6 +- .../src/encoding_selector.rs | 8 +- .../src/extension_version_selector.rs | 8 +- crates/extensions_ui/src/extensions_ui.rs | 1 - crates/file_finder/src/file_finder.rs | 16 +- crates/git_ui/src/branch_picker.rs | 6 +- crates/git_ui/src/git_graph.rs | 3 - crates/git_ui/src/picker_prompt.rs | 9 +- crates/git_ui/src/repository_selector.rs | 4 +- crates/git_ui/src/stash_picker.rs | 6 +- crates/git_ui/src/worktree_picker.rs | 8 +- .../src/language_selector.rs | 8 +- .../src/line_ending_selector.rs | 8 +- crates/onboarding/src/base_keymap_picker.rs | 8 +- .../open_path_prompt/src/open_path_prompt.rs | 3 +- .../src/open_path_prompt_tests.rs | 5 +- crates/outline/src/outline.rs | 4 +- crates/picker/src/footer.rs | 14 +- crates/picker/src/persistence.rs | 2 +- crates/picker/src/picker.rs | 209 ++++++++++++------ crates/picker/src/popover_menu.rs | 2 +- crates/picker/src/preview/render.rs | 1 - crates/picker/src/render.rs | 35 +-- crates/picker/src/shape.rs | 60 ++--- crates/project_symbols/src/project_symbols.rs | 4 +- crates/recent_projects/src/recent_projects.rs | 7 +- crates/recent_projects/src/remote_servers.rs | 18 +- .../src/sidebar_recent_projects.rs | 3 +- crates/recent_projects/src/wsl_picker.rs | 2 +- crates/repl/src/components/kernel_options.rs | 5 +- crates/search/src/text_finder.rs | 7 +- crates/search/src/text_finder/delegate.rs | 2 +- .../src/settings_profile_selector.rs | 9 +- .../settings_ui/src/components/font_picker.rs | 7 +- .../src/components/icon_theme_picker.rs | 7 +- .../src/components/ollama_model_picker.rs | 7 +- .../src/components/theme_picker.rs | 7 +- crates/snippets_ui/src/snippets_ui.rs | 8 +- crates/tab_switcher/src/tab_switcher.rs | 1 - crates/tasks_ui/src/modal.rs | 8 +- .../theme_selector/src/icon_theme_selector.rs | 6 +- crates/theme_selector/src/theme_selector.rs | 6 +- .../src/toolchain_selector.rs | 10 +- crates/vim/src/state.rs | 8 +- 61 files changed, 286 insertions(+), 374 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 4a56cb77ce2..ae25c39276e 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -451,7 +451,7 @@ "ctrl-shift-h": "search::ToggleReplace", "alt-ctrl-g": "search::ToggleRegex", "alt-ctrl-x": "search::ToggleRegex", - "ctrl-alt-p": "project_search::OpenTextFinder", + "ctrl-alt-f": "project_search::OpenTextFinder", }, }, { @@ -481,7 +481,7 @@ "ctrl-shift-h": "search::ToggleReplace", "alt-ctrl-g": "search::ToggleRegex", "alt-ctrl-x": "search::ToggleRegex", - "ctrl-alt-p": "project_search::OpenTextFinder", + "ctrl-alt-f": "project_search::OpenTextFinder", }, }, { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 175a9b7b9b9..1c16af5b819 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -508,7 +508,7 @@ "cmd-shift-h": "search::ToggleReplace", "alt-cmd-g": "search::ToggleRegex", "alt-cmd-x": "search::ToggleRegex", - "cmd-alt-p": "project_search::OpenTextFinder", + "alt-cmd-f": "project_search::OpenTextFinder", }, }, { @@ -542,7 +542,7 @@ "cmd-shift-h": "search::ToggleReplace", "alt-cmd-g": "search::ToggleRegex", "alt-cmd-x": "search::ToggleRegex", - "cmd-alt-p": "project_search::OpenTextFinder", + "alt-cmd-f": "project_search::OpenTextFinder", }, }, { diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index 50d4d83c236..442482f5aa7 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -452,7 +452,7 @@ "ctrl-shift-f": "search::FocusSearch", "ctrl-shift-h": "search::ToggleReplace", "alt-r": "search::ToggleRegex", // vscode - "ctrl-alt-p": "project_search::OpenTextFinder", + "ctrl-alt-f": "project_search::OpenTextFinder", }, }, { @@ -483,7 +483,7 @@ "escape": "project_search::ToggleFocus", "ctrl-shift-h": "search::ToggleReplace", "alt-r": "search::ToggleRegex", // vscode - "ctrl-alt-p": "project_search::OpenTextFinder", + "ctrl-alt-f": "project_search::OpenTextFinder", }, }, { diff --git a/assets/keymaps/specific-overrides-macos.json b/assets/keymaps/specific-overrides-macos.json index 9c3b95c3770..151bebeec01 100644 --- a/assets/keymaps/specific-overrides-macos.json +++ b/assets/keymaps/specific-overrides-macos.json @@ -32,7 +32,7 @@ "context": "TextFinder || (TextFinder > Picker > Editor) || (TextFinder > Picker > menu)", "use_key_equivalents": true, "bindings": { - "cmd-alt-p": "text_finder::ToProjectSearch", + "alt-cmd-f": "text_finder::ToProjectSearch", "cmd-j": "pane::SplitDown", "cmd-k": "pane::SplitUp", "cmd-h": "pane::SplitLeft", diff --git a/assets/keymaps/specific-overrides.json b/assets/keymaps/specific-overrides.json index 05ee8e93e04..283d930b86e 100644 --- a/assets/keymaps/specific-overrides.json +++ b/assets/keymaps/specific-overrides.json @@ -29,7 +29,7 @@ { "context": "TextFinder || (TextFinder > Picker > Editor) || (TextFinder > Picker > menu)", "bindings": { - "ctrl-alt-p": "text_finder::ToProjectSearch", + "ctrl-alt-f": "text_finder::ToProjectSearch", "ctrl-j": "pane::SplitDown", "ctrl-k": "pane::SplitUp", "ctrl-h": "pane::SplitLeft", diff --git a/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs b/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs index 7cafc7ad57b..c01939a31ac 100644 --- a/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs +++ b/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs @@ -295,7 +295,7 @@ impl ManageProfilesModal { window, cx, ) - .modal(false) + .embedded() }); let dismiss_subscription = cx.subscribe_in(&model_picker, window, { diff --git a/crates/agent_ui/src/agent_configuration/tool_picker.rs b/crates/agent_ui/src/agent_configuration/tool_picker.rs index 757bec6c5ed..edb510d6f7e 100644 --- a/crates/agent_ui/src/agent_configuration/tool_picker.rs +++ b/crates/agent_ui/src/agent_configuration/tool_picker.rs @@ -25,14 +25,7 @@ impl ToolPicker { window: &mut Window, cx: &mut Context, ) -> Self { - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .modal(false) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx).embedded()); Self { picker } } @@ -41,14 +34,7 @@ impl ToolPicker { window: &mut Window, cx: &mut Context, ) -> Self { - let picker = cx.new(|cx| { - Picker::list(delegate, window, cx) - .modal(false) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::list(delegate, window, cx).embedded()); Self { picker } } } diff --git a/crates/agent_ui/src/config_options.rs b/crates/agent_ui/src/config_options.rs index 4d9f218bffc..9d34aedede2 100644 --- a/crates/agent_ui/src/config_options.rs +++ b/crates/agent_ui/src/config_options.rs @@ -299,9 +299,7 @@ impl ConfigOptionSelector { Picker::nonsearchable_list(delegate, window, picker_cx) } .show_scrollbar(true) - .minimum_results_width(rems(20.)) - .height(rems(20.)) - .no_vertical_padding() + .initial_width(rems(20.)) }) }; diff --git a/crates/agent_ui/src/language_model_selector.rs b/crates/agent_ui/src/language_model_selector.rs index 5a1c9383326..7f9ff087b38 100644 --- a/crates/agent_ui/src/language_model_selector.rs +++ b/crates/agent_ui/src/language_model_selector.rs @@ -47,15 +47,12 @@ pub fn language_model_selector( if popover_styles { Picker::list(delegate, window, cx) .show_scrollbar(true) - .minimum_results_width(rems(20.)) - .height(rems(20.)) - .no_vertical_padding() - .modal(false) + .initial_width(rems(20.)) + .popover() } else { Picker::list(delegate, window, cx) .show_scrollbar(true) - .minimum_results_width(rems(20.)) - .initial_width(rems(20.)) + .embedded() } } diff --git a/crates/agent_ui/src/model_selector.rs b/crates/agent_ui/src/model_selector.rs index c0d4441b160..d177f0adc43 100644 --- a/crates/agent_ui/src/model_selector.rs +++ b/crates/agent_ui/src/model_selector.rs @@ -35,9 +35,7 @@ pub fn acp_model_selector( let delegate = ModelPickerDelegate::new(selector, focus_handle, window, cx); Picker::list(delegate, window, cx) .show_scrollbar(true) - .minimum_results_width(rems(20.)) - .height(rems(20.)) - .no_vertical_padding() + .initial_width(rems(20.)) } enum ModelPickerEntry { diff --git a/crates/agent_ui/src/profile_selector.rs b/crates/agent_ui/src/profile_selector.rs index 247890f51a4..00f989b5013 100644 --- a/crates/agent_ui/src/profile_selector.rs +++ b/crates/agent_ui/src/profile_selector.rs @@ -123,9 +123,7 @@ impl ProfileSelector { let picker = cx.new(|cx| { Picker::list(delegate, window, cx) .show_scrollbar(true) - .minimum_results_width(rems(18.)) - .height(rems(20.)) - .no_vertical_padding() + .initial_width(rems(18.)) }); self.picker = Some(picker); diff --git a/crates/agent_ui/src/threads_archive_view.rs b/crates/agent_ui/src/threads_archive_view.rs index cb961e8503d..51d2644ac73 100644 --- a/crates/agent_ui/src/threads_archive_view.rs +++ b/crates/agent_ui/src/threads_archive_view.rs @@ -1134,11 +1134,7 @@ impl ProjectPickerModal { let picker = cx.new(|cx| { Picker::list(delegate, window, cx) .list_measure_all() - .modal(false) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() + .embedded() }); let picker_focus_handle = picker.focus_handle(cx); diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index be1a5280bad..94a843d5c16 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -65,11 +65,7 @@ impl ChannelModal { window, cx, ) - .modal(false) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() + .embedded() }); Self { diff --git a/crates/collab_ui/src/collab_panel/contact_finder.rs b/crates/collab_ui/src/collab_panel/contact_finder.rs index 177058b2418..56eb7679a61 100644 --- a/crates/collab_ui/src/collab_panel/contact_finder.rs +++ b/crates/collab_ui/src/collab_panel/contact_finder.rs @@ -21,14 +21,7 @@ impl ContactFinder { potential_contacts: Arc::from([]), selected_index: 0, }; - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .modal(false) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx).embedded()); Self { picker } } diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index b744ad5e770..2b8f5a9fa94 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -125,11 +125,7 @@ impl CommandPalette { ); let picker = cx.new(|cx| { - let picker = Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding(); + let picker = Picker::uniform_list(delegate, window, cx); picker.set_query(query, window, cx); picker }); diff --git a/crates/debugger_ui/src/attach_modal.rs b/crates/debugger_ui/src/attach_modal.rs index ab7f044774b..cd7705f0e73 100644 --- a/crates/debugger_ui/src/attach_modal.rs +++ b/crates/debugger_ui/src/attach_modal.rs @@ -103,11 +103,7 @@ impl AttachModal { window, cx, ) - .modal(modal) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() + .when(!modal, |picker| picker.embedded()) }); Self { _subscription: cx.subscribe(&picker, |_, _, _, cx| { diff --git a/crates/debugger_ui/src/new_process_modal.rs b/crates/debugger_ui/src/new_process_modal.rs index ecd7d813bd5..eabcdfbd429 100644 --- a/crates/debugger_ui/src/new_process_modal.rs +++ b/crates/debugger_ui/src/new_process_modal.rs @@ -106,9 +106,7 @@ impl NewProcessModal { let delegate = DebugDelegate::new(debug_panel.downgrade(), task_store.clone()); Picker::list(delegate, window, cx) - .modal(false) - .height(rems(24.)) - .no_vertical_padding() + .embedded() .list_measure_all() }); diff --git a/crates/dev_container/src/lib.rs b/crates/dev_container/src/lib.rs index d90a06a67f3..963c5a57a29 100644 --- a/crates/dev_container/src/lib.rs +++ b/crates/dev_container/src/lib.rs @@ -1184,8 +1184,7 @@ impl StatefulModal for DevContainerModal { }), ); - let picker = - cx.new(|cx| Picker::uniform_list(delegate, window, cx).modal(false)); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx).embedded()); self.picker = Some(picker); Some(DevContainerState::TemplateQueryReturned(Ok(items))) } else { @@ -1316,8 +1315,7 @@ impl StatefulModal for DevContainerModal { }), ); - let picker = - cx.new(|cx| Picker::uniform_list(delegate, window, cx).modal(false)); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx).embedded()); self.features_picker = Some(picker); Some(DevContainerState::FeaturesQueryReturned(template_entry)) } else { diff --git a/crates/encoding_selector/src/encoding_selector.rs b/crates/encoding_selector/src/encoding_selector.rs index 7e4ac8302b5..9aa97a2a4c5 100644 --- a/crates/encoding_selector/src/encoding_selector.rs +++ b/crates/encoding_selector/src/encoding_selector.rs @@ -95,13 +95,7 @@ impl EncodingSelector { fn new(buffer: Entity, window: &mut Window, cx: &mut Context) -> Self { let delegate = EncodingSelectorDelegate::new(cx.entity().downgrade(), buffer); - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(gpui::rems(34.)) - .minimum_results_width(gpui::rems(34.)) - .height(gpui::rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx)); Self { picker } } } diff --git a/crates/extensions_ui/src/extension_version_selector.rs b/crates/extensions_ui/src/extension_version_selector.rs index 7e599bf371c..1ab1bd659f9 100644 --- a/crates/extensions_ui/src/extension_version_selector.rs +++ b/crates/extensions_ui/src/extension_version_selector.rs @@ -40,13 +40,7 @@ impl ExtensionVersionSelector { window: &mut Window, cx: &mut Context, ) -> Self { - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx)); Self { picker } } } diff --git a/crates/extensions_ui/src/extensions_ui.rs b/crates/extensions_ui/src/extensions_ui.rs index 22e13b54c9b..079512e901c 100644 --- a/crates/extensions_ui/src/extensions_ui.rs +++ b/crates/extensions_ui/src/extensions_ui.rs @@ -276,7 +276,6 @@ pub fn init(cx: &mut App) { workspace.toggle_modal(window, cx, |window, cx| { let delegate = DevExtensionRebuildPickerDelegate::new(dev_extensions); Picker::uniform_list(delegate, window, cx) - .minimum_results_width(rems(34.)) }); } } diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index f609dae7808..25a2e5ab0e1 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -169,20 +169,10 @@ impl FileFinder { let modal_max_width_setting = FileFinderSettings::get_global(cx).modal_max_width; let project = delegate.project.clone(); + let modal_max_width = Self::modal_max_width(modal_max_width_setting, window); let picker = cx.new(|cx| { - let picker = Picker::uniform_list_with_preview(delegate, project, window, cx) - .minimum_results_width(gpui::rems(34.)) - .height(gpui::rems(24.)) - .no_vertical_padding(); - // The dedicated file finder width setting has been removed in favor of - // persisted picker sizes. For migration we still honor it as the initial - // width, but only if the user actually changed it from the default; - if modal_max_width_setting != FileFinderWidth::default() { - let modal_max_width = Self::modal_max_width(modal_max_width_setting, window); - picker.initial_width(Rems::from_pixels(modal_max_width, window)) - } else { - picker - } + Picker::uniform_list_with_preview(delegate, project, window, cx) + .initial_width(Rems::from_pixels(modal_max_width, window)) }); let picker_focus_handle = picker.focus_handle(cx); picker.update(cx, |picker, _| { diff --git a/crates/git_ui/src/branch_picker.rs b/crates/git_ui/src/branch_picker.rs index 43158a32b2a..1cbfcca4364 100644 --- a/crates/git_ui/src/branch_picker.rs +++ b/crates/git_ui/src/branch_picker.rs @@ -277,11 +277,9 @@ impl BranchList { let picker = cx.new(|cx| { Picker::uniform_list(delegate, window, cx) - .minimum_results_width(width) - .height(rems(24.)) - .no_vertical_padding() + .initial_width(width) .show_scrollbar(true) - .modal(!embedded) + .when(embedded, |picker| picker.embedded()) }); let picker_focus_handle = picker.focus_handle(cx); diff --git a/crates/git_ui/src/git_graph.rs b/crates/git_ui/src/git_graph.rs index 84628d0dd61..384474e2105 100644 --- a/crates/git_ui/src/git_graph.rs +++ b/crates/git_ui/src/git_graph.rs @@ -111,9 +111,6 @@ impl CommitTagPicker { let picker = cx.new(|cx| { Picker::nonsearchable_uniform_list(delegate, window, cx) .initial_width(COMMIT_TAG_LIST_WIDTH_IN_REMS) - .minimum_results_width(COMMIT_TAG_LIST_WIDTH_IN_REMS) - .height(rems(24.)) - .no_vertical_padding() }); Self { picker } } diff --git a/crates/git_ui/src/picker_prompt.rs b/crates/git_ui/src/picker_prompt.rs index 03c0553fef8..554d706e942 100644 --- a/crates/git_ui/src/picker_prompt.rs +++ b/crates/git_ui/src/picker_prompt.rs @@ -55,13 +55,8 @@ impl PickerPrompt { window: &mut Window, cx: &mut Context, ) -> Self { - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(rems(rem_width)) - .minimum_results_width(rems(rem_width)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = + cx.new(|cx| Picker::uniform_list(delegate, window, cx).initial_width(rems(rem_width))); let _subscription = cx.subscribe(&picker, |_, _, _, cx| cx.emit(DismissEvent)); Self { picker, diff --git a/crates/git_ui/src/repository_selector.rs b/crates/git_ui/src/repository_selector.rs index 54303bca6b0..19c653cdaa0 100644 --- a/crates/git_ui/src/repository_selector.rs +++ b/crates/git_ui/src/repository_selector.rs @@ -64,9 +64,7 @@ impl RepositorySelector { let picker = cx.new(|cx| { Picker::uniform_list(delegate, window, cx) - .minimum_results_width(width) - .height(rems(20.)) - .no_vertical_padding() + .initial_width(width) .show_scrollbar(true) }); diff --git a/crates/git_ui/src/stash_picker.rs b/crates/git_ui/src/stash_picker.rs index feaaaf7ab9f..85a0206ddd0 100644 --- a/crates/git_ui/src/stash_picker.rs +++ b/crates/git_ui/src/stash_picker.rs @@ -128,11 +128,9 @@ impl StashList { let delegate = StashListDelegate::new(repository, workspace, window, cx); let picker = cx.new(|cx| { Picker::uniform_list(delegate, window, cx) - .minimum_results_width(width) - .height(rems(24.)) - .no_vertical_padding() + .initial_width(width) .show_scrollbar(true) - .modal(!embedded) + .when(embedded, |picker| picker.embedded()) }); let picker_focus_handle = picker.focus_handle(cx); picker.update(cx, |picker, _| { diff --git a/crates/git_ui/src/worktree_picker.rs b/crates/git_ui/src/worktree_picker.rs index dab2b3e00b5..df0e8992751 100644 --- a/crates/git_ui/src/worktree_picker.rs +++ b/crates/git_ui/src/worktree_picker.rs @@ -8,7 +8,7 @@ use git::repository::Worktree as GitWorktree; use gpui::{ Action, AnyElement, App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement, Modifiers, ModifiersChangedEvent, ParentElement, PromptLevel, - Render, SharedString, Styled, Subscription, Task, TaskExt, WeakEntity, Window, actions, rems, + Render, SharedString, Styled, Subscription, Task, TaskExt, WeakEntity, Window, actions, }; use picker::{Picker, PickerDelegate, PickerEditorPosition}; use project::Project; @@ -128,11 +128,7 @@ impl WorktreePicker { Picker::list(delegate, window, cx) .list_measure_all() .show_scrollbar(true) - .modal(false) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(20.)) - .no_vertical_padding() + .embedded() }); let picker_focus_handle = picker.focus_handle(cx); diff --git a/crates/language_selector/src/language_selector.rs b/crates/language_selector/src/language_selector.rs index f1d86da079c..3fab580ae49 100644 --- a/crates/language_selector/src/language_selector.rs +++ b/crates/language_selector/src/language_selector.rs @@ -83,13 +83,7 @@ impl LanguageSelector { current_language_name, ); - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx)); Self { picker } } } diff --git a/crates/line_ending_selector/src/line_ending_selector.rs b/crates/line_ending_selector/src/line_ending_selector.rs index 5ba42c30827..f3298efee35 100644 --- a/crates/line_ending_selector/src/line_ending_selector.rs +++ b/crates/line_ending_selector/src/line_ending_selector.rs @@ -65,13 +65,7 @@ impl LineEndingSelector { let line_ending = buffer.read(cx).line_ending(); let delegate = LineEndingSelectorDelegate::new(cx.entity().downgrade(), buffer, project, line_ending); - let picker = cx.new(|cx| { - Picker::nonsearchable_uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::nonsearchable_uniform_list(delegate, window, cx)); Self { picker } } } diff --git a/crates/onboarding/src/base_keymap_picker.rs b/crates/onboarding/src/base_keymap_picker.rs index 6f0fa5d49fc..351c87a96bf 100644 --- a/crates/onboarding/src/base_keymap_picker.rs +++ b/crates/onboarding/src/base_keymap_picker.rs @@ -61,13 +61,7 @@ impl BaseKeymapSelector { window: &mut Window, cx: &mut Context, ) -> Self { - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx)); Self { picker } } } diff --git a/crates/open_path_prompt/src/open_path_prompt.rs b/crates/open_path_prompt/src/open_path_prompt.rs index 7214ba570e4..6fc35e697c8 100644 --- a/crates/open_path_prompt/src/open_path_prompt.rs +++ b/crates/open_path_prompt/src/open_path_prompt.rs @@ -233,8 +233,7 @@ impl OpenPathPrompt { workspace.toggle_modal(window, cx, |window, cx| { let delegate = OpenPathDelegate::new(tx, lister.clone(), creating_path, cx).show_hidden(); - let picker = - Picker::uniform_list(delegate, window, cx).minimum_results_width(rems(34.)); + let picker = Picker::uniform_list(delegate, window, cx); let mut query = lister.default_query(cx); if let Some(suggested_name) = suggested_name { query.push_str(&suggested_name); diff --git a/crates/open_path_prompt/src/open_path_prompt_tests.rs b/crates/open_path_prompt/src/open_path_prompt_tests.rs index 51eb33b66e7..6254be253f8 100644 --- a/crates/open_path_prompt/src/open_path_prompt_tests.rs +++ b/crates/open_path_prompt/src/open_path_prompt_tests.rs @@ -5,7 +5,6 @@ use gpui::{AppContext, Entity, TestAppContext, VisualTestContext}; use picker::{Picker, PickerDelegate}; use project::Project; use serde_json::json; -use ui::rems; use util::path; use workspace::{AppState, MultiWorkspace}; @@ -509,9 +508,7 @@ fn build_open_path_prompt( delegate }; cx.new(|cx| { - let picker = Picker::uniform_list(delegate, window, cx) - .minimum_results_width(rems(34.)) - .modal(false); + let picker = Picker::uniform_list(delegate, window, cx).embedded(); let query = lister.default_query(cx); picker.set_query(&query, window, cx); picker diff --git a/crates/outline/src/outline.rs b/crates/outline/src/outline.rs index f1badca2ea2..ffd11e3197c 100644 --- a/crates/outline/src/outline.rs +++ b/crates/outline/src/outline.rs @@ -179,12 +179,10 @@ impl OutlineView { let delegate = OutlineViewDelegate::new(cx.entity().downgrade(), outline, editor, cx); let picker = cx.new(|cx| { Picker::uniform_list(delegate, window, cx) - .minimum_results_width(rems(34.)) - .height(Rems::from_pixels( + .max_height(Rems::from_pixels( window.viewport_size().height * 0.75, window, )) - .no_vertical_padding() .show_scrollbar(true) }); OutlineView { picker } diff --git a/crates/picker/src/footer.rs b/crates/picker/src/footer.rs index c3bb4d2200d..fc90c5f2530 100644 --- a/crates/picker/src/footer.rs +++ b/crates/picker/src/footer.rs @@ -127,14 +127,12 @@ impl Picker { cx: &mut Context, ) -> impl IntoElement { let focus_handle = self.focus_handle(cx); - let toggle_focus_handle = focus_handle.clone(); let right_focus_handle = focus_handle.clone(); let below_focus_handle = focus_handle.clone(); let current = self.preview_layout().unwrap_or(preview::Layout::Hidden); let preview_visible = current != preview::Layout::Hidden; h_flex() - .gap_1() .child( Button::new("picker-preview-toggle", "Preview") .when(preview_visible, |this| this.color(Color::Accent)) @@ -142,22 +140,12 @@ impl Picker { KeyBinding::for_action_in(&TogglePreview, &focus_handle, cx) .size(rems_from_px(12.)), ) - .tooltip(move |_window, cx| { - Tooltip::for_action_in( - "Toggle Preview", - &TogglePreview, - &toggle_focus_handle, - cx, - ) - }) .on_click( cx.listener(|this, _, window, cx| this.toggle_preview_visible(window, cx)), ), ) - // The layout buttons (preview to the right / below) are only relevant - // once the preview is showing, so hide them while it's hidden. .when(preview_visible, |this| { - this.child(div().child(Divider::vertical().color(ui::DividerColor::Border))) + this.child(Divider::vertical().h_4().mx_1()) .child( IconButton::new("picker-preview-right", IconName::DiffSplit) .icon_size(IconSize::Small) diff --git a/crates/picker/src/persistence.rs b/crates/picker/src/persistence.rs index 3e26d5fe10b..ab61859abd8 100644 --- a/crates/picker/src/persistence.rs +++ b/crates/picker/src/persistence.rs @@ -6,7 +6,7 @@ use ui::Window; use crate::preview; use crate::shape::{self, Centered, RelativeHeight, RelativeWidth, Shape, ViewportFraction}; -const PICKERS_NAMESPACE: &str = "pickers"; +const PICKERS_NAMESPACE: &str = "pickers_v2"; pub(crate) fn store_shape_for_this_layout( picker_delegate: &'static str, diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 93d54e10a78..c4579781ade 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -38,6 +38,9 @@ pub use preview::PreviewSource; pub use preview::Update as PreviewUpdate; pub use ui_input::ErasedEditor; +pub const DEFAULT_MODAL_WIDTH: Rems = Rems(34.0); +pub const DEFAULT_MODAL_MAX_HEIGHT: Rems = Rems(24.0); + enum ElementContainer { List(ListState), UniformList(UniformListScrollHandle), @@ -88,6 +91,30 @@ struct PendingUpdateMatches { _task: Task>, } +#[derive(Clone, Copy)] +enum Presentation { + /// A self-contained modal: draws its own elevated background and dismisses + /// when it loses focus. May optionally be resized (persisting its size); + /// resizing only makes sense for modals. + Modal { resizable: bool }, + /// A popover attached to a menu/trigger: draws its own elevated background + /// and dismisses when it loses focus, but is never resizable. + Popover, + /// Embedded inside a larger container (e.g. another modal) that provides its + /// own chrome and handles dismissal. + Embedded, +} + +/// The default size for a given preview layout. With the preview hidden the +/// picker uses its standard size; showing a preview expands it to the larger +/// "telescope" size so the results pane isn't cramped beside the preview. +fn default_shape_for_layout(hidden: shape::Centered, layout: preview::Layout) -> shape::Centered { + match layout { + preview::Layout::Hidden => hidden, + preview::Layout::Right | preview::Layout::Below => shape::Centered::default(), + } +} + pub struct Picker { pub delegate: D, element_container: ElementContainer, @@ -96,16 +123,14 @@ pub struct Picker { pending_update_matches: Option, confirm_on_update: Option, shape: shape::Shape, - /// set through [Picker::width] and [Picker::height] + /// The size the picker opens at (and resets to). Defaults depend on whether + /// the picker has a preview; see [`Picker::initial_width`] / [`Picker::max_height`]. default_shape: shape::Centered, - vertical_padding: shape::VerticalPadding, size_bounds: shape::SizeBounds, /// An external control to display a scrollbar in the `Picker`. show_scrollbar: bool, - /// Whether the `Picker` is rendered as a self-contained modal. - /// - /// Set this to `false` when rendering the `Picker` as part of a larger modal. - is_modal: bool, + /// How the picker is presented, which controls its chrome and behavior. + presentation: Presentation, /// Bounds tracking for the picker container (for aside positioning) picker_bounds: Rc>>>, /// Bounds tracking for items (for aside positioning) - maps item index to bounds @@ -474,9 +499,27 @@ impl Picker { .flatten() .unwrap_or_default(); }; - let shape = persistence::try_load_shape(D::name(), preview.as_ref().map(|p| p.layout), cx) - .log_err() - .flatten(); + let has_preview = preview.is_some(); + let persisted_shape = + persistence::try_load_shape(D::name(), preview.as_ref().map(|p| p.layout), cx) + .log_err() + .flatten(); + // Every picker opens at the standard "simple" size: a fixed width and a + // standard max height it shrinks below when there's little content. + // Showing a preview expands it to the larger "telescope" size (see + // `default_shape_for_layout`). + let default_shape = shape::Centered::simple(); + let initial_layout = preview + .as_ref() + .map(|p| p.layout) + .unwrap_or(preview::Layout::Hidden); + let mut size_bounds = shape::SizeBounds::default(); + // For a plain picker the whole-picker minimum is just its opening width, + // so it can't be resized/clamped narrower than it opens. Preview pickers + // keep the standard per-pane minimums. + if !has_preview && let Some(width) = default_shape.width.as_rems() { + size_bounds.min_results.width = width; + } let mut this = Self { delegate, head, @@ -484,15 +527,21 @@ impl Picker { pending_update_matches: None, confirm_on_update: None, preview, - shape_loaded_from_persistence: shape.is_some(), - shape: shape.unwrap_or_default(), - default_shape: shape::Centered::default(), - vertical_padding: shape::VerticalPadding::default(), + shape_loaded_from_persistence: persisted_shape.is_some(), + shape: persisted_shape.unwrap_or_else(|| { + shape::Shape::HorizontallyCentered(default_shape_for_layout( + default_shape, + initial_layout, + )) + }), + default_shape, show_scrollbar: false, - is_modal: true, + presentation: Presentation::Modal { + resizable: has_preview, + }, picker_bounds: Rc::new(Cell::new(None)), item_bounds: Rc::new(RefCell::new(HashMap::default())), - size_bounds: shape::SizeBounds::default(), + size_bounds, actions_menu_handle: PopoverMenuHandle::default(), }; this.update_matches("".to_string(), window, cx); @@ -513,34 +562,33 @@ impl Picker { } } - /// Sets the width the picker appears with if the user has never resized it - /// or when the user sets it back to it's default size. + /// Overrides the width the picker opens at (and resets to). Plain pickers + /// default to [`DEFAULT_MODAL_WIDTH`]; only call this for pickers that need a + /// different width (e.g. narrow popover selectors). + /// + /// For a plain (no-preview) picker the opening width is also its minimum, so + /// it can't be resized or clamped narrower than it opens. pub fn initial_width(mut self, width: impl Into) -> Self { let width = width.into(); self.default_shape.width = width; if !self.shape_loaded_from_persistence { self.shape.set_initial_width(width); } + // A plain picker's whole-picker minimum tracks its opening width. Preview + // pickers keep the standard per-pane minimums. + if self.preview.is_none() + && let Some(rems) = width.as_rems() + { + self.size_bounds.min_results.width = rems; + } self } - /// Sets the minimum width, the picker can not be resized smaller then this. - /// Leave unset to use sane defaults. - /// - /// This applies to the results. If there is no preview that is the whole picker. - pub fn minimum_results_width(mut self, width: impl Into) -> Self { - self.size_bounds.min_results.width = width.into(); - self - } - - /// Sets the width the picker appears with if the user has never resized it - /// or when the user sets it back to it's default size. - /// - /// # Padding - /// By default the picker will fill this space. If you want it to only grow - /// as large as it needs and treat the height as a bound use - /// [`no_vertical_padding`] - pub fn height(mut self, height: impl Into) -> Self { + /// Overrides the picker's max height. Plain pickers default to + /// [`DEFAULT_MODAL_MAX_HEIGHT`] and shrink below it to fit their content; + /// only call this for pickers that want a different cap (e.g. the outline + /// view, which is taller). + pub fn max_height(mut self, height: impl Into) -> Self { let height = height.into(); self.default_shape.height = height; if !self.shape_loaded_from_persistence { @@ -549,23 +597,12 @@ impl Picker { self } - /// Makes the picker shrink to fit its content rather than padding out to its - /// full height when there are fewer results than fit. - pub fn no_vertical_padding(mut self) -> Self { - self.vertical_padding = shape::VerticalPadding::None; - self - } - - fn vertical_padding(&self) -> shape::VerticalPadding { - let preview_visible = self - .preview + /// Whether the picker fills its full height (preview visible) or shrinks to + /// fit its content, treating the height as a maximum (no preview visible). + fn fill_height(&self) -> bool { + self.preview .as_ref() - .is_some_and(|preview| preview.layout != preview::Layout::Hidden); - if preview_visible { - shape::VerticalPadding::Pad - } else { - self.vertical_padding - } + .is_some_and(|preview| preview.layout != preview::Layout::Hidden) } pub fn show_scrollbar(mut self, show_scrollbar: bool) -> Self { @@ -573,11 +610,50 @@ impl Picker { self } - pub fn modal(mut self, modal: bool) -> Self { - self.is_modal = modal; + /// Presents the picker as embedded inside a larger container that provides + /// its own chrome and dismissal, rather than the default self-contained + /// modal. Use [`popover`](Self::popover) instead for menu-attached pickers. + pub fn embedded(mut self) -> Self { + self.presentation = Presentation::Embedded; self } + /// Presents the picker as a popover surface: it draws its own elevated + /// background (like a modal) but is not resizable. Use this for pickers + /// shown inside a menu/popover. + pub fn popover(mut self) -> Self { + self.set_popover(); + self + } + + pub(crate) fn set_popover(&mut self) { + self.presentation = Presentation::Popover; + } + + /// Controls whether the user can drag to resize the picker (and whether its + /// size is persisted). Only applies to modal pickers; no-op otherwise. + /// Defaults to `true` for pickers with a preview and `false` otherwise. + pub fn resizable(mut self, resizable: bool) -> Self { + if let Presentation::Modal { resizable: r } = &mut self.presentation { + *r = resizable; + } + self + } + + /// Whether the picker draws its own elevated background and dismisses on + /// blur (modals and popovers, but not embedded pickers). + fn draws_own_container(&self) -> bool { + matches!( + self.presentation, + Presentation::Modal { .. } | Presentation::Popover + ) + } + + /// Whether the picker can be resized (only ever true for modals). + fn is_resizable(&self) -> bool { + matches!(self.presentation, Presentation::Modal { resizable: true }) + } + pub fn list_measure_all(mut self) -> Self { match self.element_container { ElementContainer::List(state) => { @@ -888,7 +964,7 @@ impl Picker { let menu_focused = self.actions_menu_handle.is_focused(window, cx) || self.actions_menu_handle.is_deployed() || self.delegate.has_another_open_menu(window, cx); - if self.is_modal && window.is_window_active() && !menu_focused { + if self.draws_own_container() && window.is_window_active() && !menu_focused { self.cancel(&menu::Cancel, window, cx); } } @@ -1107,12 +1183,13 @@ impl Picker { } fn render_element_container(&self, cx: &mut Context) -> impl IntoElement { - // When the picker shrinks to fit content (`None`), the list infers its - // size from its items. When the picker pads to its full height (`Pad`), - // the list fills the available space. - let sizing_behavior = match self.vertical_padding() { - shape::VerticalPadding::None => ListSizingBehavior::Infer, - shape::VerticalPadding::Pad => ListSizingBehavior::Auto, + // When the picker shrinks to fit its content, the list infers its size + // from its items. When it fills its full height (preview visible), the + // list fills the available space. + let sizing_behavior = if self.fill_height() { + ListSizingBehavior::Auto + } else { + ListSizingBehavior::Infer }; match &self.element_container { @@ -1179,12 +1256,17 @@ impl Picker { return; }; preview.layout = layout; - if let Some(previously_resized) = persistence::try_load_shape(D::name(), layout, cx) + // Restore the size the user last left this layout at, or fall back to the + // layout's default (simple when hidden, larger when a preview is shown). + self.shape = persistence::try_load_shape(D::name(), layout, cx) .log_err() .flatten() - { - self.shape = previously_resized; - } + .unwrap_or_else(|| { + shape::Shape::HorizontallyCentered(default_shape_for_layout( + self.default_shape, + layout, + )) + }); self.delegate .preview_layout_changed(matches!(layout, preview::Layout::Right)); cx.notify(); @@ -1366,3 +1448,4 @@ mod tests { impl EventEmitter for Picker {} impl ModalView for Picker {} +impl ui::FluentBuilder for Picker {} diff --git a/crates/picker/src/popover_menu.rs b/crates/picker/src/popover_menu.rs index b3c4fba41a4..e7fbec87002 100644 --- a/crates/picker/src/popover_menu.rs +++ b/crates/picker/src/popover_menu.rs @@ -36,7 +36,7 @@ where anchor: Anchor, cx: &mut App, ) -> Self { - picker.update(cx, |picker, _| picker.is_modal = false); + picker.update(cx, |picker, _| picker.set_popover()); Self { _subscriptions: vec![cx.subscribe(&picker, |picker, &DismissEvent, cx| { picker.update(cx, |_, cx| cx.emit(DismissEvent)); diff --git a/crates/picker/src/preview/render.rs b/crates/picker/src/preview/render.rs index 4733c8ebf5b..aac1d0b96bb 100644 --- a/crates/picker/src/preview/render.rs +++ b/crates/picker/src/preview/render.rs @@ -37,7 +37,6 @@ impl EditorPreview { if let Some(message) = &self.message { self.render_message(message, cx).into_any_element() } else { - debug_assert!(!self.preview_editor.read(cx).is_empty(cx)); div() .flex_1() .overflow_hidden() diff --git a/crates/picker/src/render.rs b/crates/picker/src/render.rs index 7dd9bed62a0..82cdc4ad474 100644 --- a/crates/picker/src/render.rs +++ b/crates/picker/src/render.rs @@ -54,22 +54,23 @@ impl Render for Picker { // off. let has_preview = self.preview.is_some(); let content = div() - .when(self.is_modal, |this| this.elevation_3(cx)) + .when(self.draws_own_container(), |this| this.elevation_3(cx)) .when(has_preview, |this| this.overflow_hidden()) .child(content); let layout = self.preview_layout().unwrap_or(Layout::Hidden); - // Embedded pickers are not resizable - div().relative().child(content).when(self.is_modal, |this| { - // While resizing offset the (parent-centered) container - this.left(self.shape.horizontal_offset(window)) - .child(self.render_resize(Left, window, cx)) - .child(self.render_resize(Right(layout), window, cx)) - .child(self.render_resize(Bottom(layout), window, cx)) - .child(self.render_resize(LeftCorner(layout), window, cx)) - .child(self.render_resize(RightCorner(layout), window, cx)) - }) + div() + .relative() + .child(content) + .when(self.is_resizable(), |this| { + this.left(self.shape.horizontal_offset(window)) + .child(self.render_resize(Left, window, cx)) + .child(self.render_resize(Right(layout), window, cx)) + .child(self.render_resize(Bottom(layout), window, cx)) + .child(self.render_resize(LeftCorner(layout), window, cx)) + .child(self.render_resize(RightCorner(layout), window, cx)) + }) } } @@ -95,7 +96,7 @@ impl Picker { self.shape.apply_results_size( self.preview_layout(), &self.size_bounds, - self.vertical_padding(), + self.fill_height(), this, window, ) @@ -154,7 +155,7 @@ impl Picker { .when_some( self.shape.results_max_height( &self.size_bounds, - self.vertical_padding(), + self.fill_height(), window, ), |this, max_height| this.max_h(max_height), @@ -300,7 +301,9 @@ impl Picker { .child(preview.render(cx)), ), ) - .child(self.render_resize(window_controls::Middle(preview.layout), window, cx)) + .when(self.is_resizable(), |this| { + this.child(self.render_resize(window_controls::Middle(preview.layout), window, cx)) + }) } pub(crate) fn render_with_preview_right( @@ -342,7 +345,9 @@ impl Picker { .child(preview.render(cx)), ), ) - .child(self.render_resize(Middle(Layout::Right), window, cx)) + .when(self.is_resizable(), |this| { + this.child(self.render_resize(Middle(Layout::Right), window, cx)) + }) } fn finish_any_completed_resize( diff --git a/crates/picker/src/shape.rs b/crates/picker/src/shape.rs index 04c985cb331..d37e2b0aae5 100644 --- a/crates/picker/src/shape.rs +++ b/crates/picker/src/shape.rs @@ -72,6 +72,13 @@ macro_rules! relative_size { } } + /// Returns this size as [`Rems`] when it has no viewport-relative + /// component. Used to derive a rems-based minimum from an initial + /// size without needing a [`Window`]. + pub fn as_rems(&self) -> Option { + (self.viewport_fraction == 0.0).then_some(self.rems) + } + pub fn as_viewport_fraction(&self, window: &Window) -> ViewportFraction { ViewportFraction( self.viewport_fraction @@ -181,15 +188,6 @@ impl std::ops::Mul for ViewportFraction { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -pub(crate) enum VerticalPadding { - /// The picker always fills its height even if there are no results - #[default] - Pad, - /// Picker might be shorter then it's height if there is not enough to display - None, -} - #[derive(Debug, Clone, Copy)] pub(crate) struct Centered { pub(crate) width: RelativeWidth, @@ -197,6 +195,19 @@ pub(crate) struct Centered { pub(crate) preview_size: ViewportFraction, } +impl Centered { + /// The default size for a plain picker (no preview): a fixed standard width + /// and a standard *max* height that the picker shrinks below when it has + /// little content. + pub(crate) fn simple() -> Self { + Centered { + width: RelativeWidth::rems(crate::DEFAULT_MODAL_WIDTH), + height: RelativeHeight::rems(crate::DEFAULT_MODAL_MAX_HEIGHT), + preview_size: ViewportFraction::ZERO, + } + } +} + #[derive(Debug, Clone, Copy)] pub(crate) enum Shape { Resizing(PositionAndShape), @@ -211,7 +222,6 @@ pub(crate) enum Shape { pub struct SizeBounds { pub(crate) max_width: RelativeWidth, pub(crate) max_height: RelativeHeight, - /// Minimum size of the results pane. pub(crate) min_results: Size, /// Minimum size of the preview pane. Along the split axis this only needs to /// be large enough to grab the divider and shrink it back. @@ -487,7 +497,7 @@ impl Shape { &self, layout: impl Into>, bounds: &SizeBounds, - vertical_padding: VerticalPadding, + fill_height: bool, div: Div, window: &Window, ) -> Div { @@ -500,29 +510,27 @@ impl Shape { _ => full.right - full.left, }; let div = div.w(width); - match vertical_padding { - VerticalPadding::None => div, - VerticalPadding::Pad => { - let height = match layout { - Some(Layout::Below) => (full.bottom - full.top) - full.preview, - _ => full.bottom - full.top, - }; - div.h(height) - } + if fill_height { + let height = match layout { + Some(Layout::Below) => (full.bottom - full.top) - full.preview, + _ => full.bottom - full.top, + }; + div.h(height) + } else { + div } } pub(crate) fn results_max_height( &self, bounds: &SizeBounds, - vertical_padding: VerticalPadding, + fill_height: bool, window: &Window, ) -> Option { - match vertical_padding { - // No preview when results size themselves (no vertical padding), so - // clamp against the results-only minimum. - VerticalPadding::None => Some(self.height(None, bounds, window)), - VerticalPadding::Pad => None, + if fill_height { + None + } else { + Some(self.height(None, bounds, window)) } } diff --git a/crates/project_symbols/src/project_symbols.rs b/crates/project_symbols/src/project_symbols.rs index 3681d84bc81..5e4422a6fb7 100644 --- a/crates/project_symbols/src/project_symbols.rs +++ b/crates/project_symbols/src/project_symbols.rs @@ -2,7 +2,7 @@ use editor::{Bias, Editor, SelectionEffects, scroll::Autoscroll, styled_runs_for use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ App, Context, DismissEvent, Entity, HighlightStyle, ParentElement, StyledText, Task, TaskExt, - TextStyle, WeakEntity, Window, relative, rems, + TextStyle, WeakEntity, Window, relative, }; use ordered_float::OrderedFloat; use picker::{Picker, PickerDelegate}; @@ -26,7 +26,7 @@ pub fn init(cx: &mut App) { let handle = cx.entity().downgrade(); workspace.toggle_modal(window, cx, move |window, cx| { let delegate = ProjectSymbolsDelegate::new(handle, project); - Picker::uniform_list(delegate, window, cx).minimum_results_width(rems(34.)) + Picker::uniform_list(delegate, window, cx) }) }, ); diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 4d0b237964c..696817d10cc 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -632,9 +632,7 @@ impl RecentProjects { let picker = cx.new(|cx| { Picker::list(delegate, window, cx) .list_measure_all() - .minimum_results_width(rems(rem_width)) - .height(rems(24.)) - .no_vertical_padding() + .initial_width(rems(rem_width)) .show_scrollbar(true) }); @@ -2573,8 +2571,7 @@ mod tests { Picker::list(delegate, window, cx) .list_measure_all() .show_scrollbar(true) - .height(Rems::from_pixels(px(240.0), window)) - .no_vertical_padding() + .max_height(Rems::from_pixels(px(240.0), window)) }); draw(cx); (picker, cx) diff --git a/crates/recent_projects/src/remote_servers.rs b/crates/recent_projects/src/remote_servers.rs index 9e6653b43d7..fc883849ecb 100644 --- a/crates/recent_projects/src/remote_servers.rs +++ b/crates/recent_projects/src/remote_servers.rs @@ -132,7 +132,7 @@ impl AddWslDistro { use crate::wsl_picker::{WslDistroSelected, WslPickerDelegate, WslPickerDismissed}; let delegate = WslPickerDelegate::new(); - let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx).modal(false)); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx).embedded()); cx.subscribe_in( &picker, @@ -398,12 +398,7 @@ impl ProjectPicker { let delegate = open_path_prompt::OpenPathDelegate::new(tx, lister, false, cx).show_hidden(); let picker = cx.new(|cx| { - let picker = Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - .modal(false); + let picker = Picker::uniform_list(delegate, window, cx).embedded(); picker.set_query(&home_dir.to_string(), window, cx); picker }); @@ -1457,7 +1452,7 @@ impl RemoteServerProjects { if configs.len() > 1 { let delegate = DevContainerPickerDelegate::new(configs, cx.weak_entity()); this.dev_container_picker = - Some(cx.new(|cx| Picker::uniform_list(delegate, window, cx).modal(false))); + Some(cx.new(|cx| Picker::uniform_list(delegate, window, cx).embedded())); } else if let Some(context) = dev_container_context { let config = configs.into_iter().next(); this.open_dev_container(config, app_state, context, window, cx); @@ -1506,10 +1501,7 @@ impl RemoteServerProjects { true, cx, ); - Picker::list(delegate, window, cx) - .modal(false) - .initial_width(rems(34.)) - .height(rems(24.)) + Picker::list(delegate, window, cx).embedded() }); let mut read_ssh_config = RemoteSettings::get_global(cx).read_ssh_config; let ssh_config_updates = if read_ssh_config { @@ -2217,7 +2209,7 @@ impl RemoteServerProjects { if configs.len() > 1 { let delegate = DevContainerPickerDelegate::new(configs, cx.weak_entity()); self.dev_container_picker = - Some(cx.new(|cx| Picker::uniform_list(delegate, window, cx).modal(false))); + Some(cx.new(|cx| Picker::uniform_list(delegate, window, cx).embedded())); let state = CreateRemoteDevContainer::new(DevContainerCreationProgress::SelectingConfig, cx); diff --git a/crates/recent_projects/src/sidebar_recent_projects.rs b/crates/recent_projects/src/sidebar_recent_projects.rs index a5cc8e14c8e..8955e63793f 100644 --- a/crates/recent_projects/src/sidebar_recent_projects.rs +++ b/crates/recent_projects/src/sidebar_recent_projects.rs @@ -56,8 +56,7 @@ impl SidebarRecentProjects { .list_measure_all() .show_scrollbar(true) .initial_width(rems(18.)) - .minimum_results_width(rems(18.)) - .modal(false) + .popover() }); let picker_focus_handle = picker.focus_handle(cx); diff --git a/crates/recent_projects/src/wsl_picker.rs b/crates/recent_projects/src/wsl_picker.rs index 6edf8a26dfb..271be7d0f51 100644 --- a/crates/recent_projects/src/wsl_picker.rs +++ b/crates/recent_projects/src/wsl_picker.rs @@ -203,7 +203,7 @@ impl WslOpenModal { cx: &mut Context, ) -> Self { let delegate = WslPickerDelegate::new(); - let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx).modal(false)); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx).embedded()); let selected = cx.subscribe_in( &picker, diff --git a/crates/repl/src/components/kernel_options.rs b/crates/repl/src/components/kernel_options.rs index 698428f92ab..cdc2a586ebf 100644 --- a/crates/repl/src/components/kernel_options.rs +++ b/crates/repl/src/components/kernel_options.rs @@ -481,10 +481,7 @@ where let picker_view = cx.new(|cx| { Picker::list(delegate, window, cx) .list_measure_all() - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - .modal(false) + .popover() }); PopoverMenu::new("kernel-switcher") diff --git a/crates/search/src/text_finder.rs b/crates/search/src/text_finder.rs index c2a8c58b633..4852e2e2386 100644 --- a/crates/search/src/text_finder.rs +++ b/crates/search/src/text_finder.rs @@ -9,7 +9,7 @@ use picker::Picker; use project::ProjectPath; use text::Anchor; -use ui::{Rems, Window}; +use ui::Window; use workspace::{DismissDecision, ModalView, Workspace}; mod delegate; @@ -223,10 +223,7 @@ impl TextFinder { fn new(delegate: Delegate, window: &mut Window, cx: &mut Context) -> Self { let project = delegate.project(cx).clone(); - let picker = cx.new(|cx| { - Picker::list_with_preview(delegate, project, window, cx) - .minimum_results_width(Rems(20.0)) - }); + let picker = cx.new(|cx| Picker::list_with_preview(delegate, project, window, cx)); let picker_weak = picker.downgrade(); let picker_focus_handle = picker.focus_handle(cx); picker.update(cx, |picker, cx| { diff --git a/crates/search/src/text_finder/delegate.rs b/crates/search/src/text_finder/delegate.rs index e0ecf2c6b6c..5fb075d314f 100644 --- a/crates/search/src/text_finder/delegate.rs +++ b/crates/search/src/text_finder/delegate.rs @@ -612,7 +612,7 @@ impl PickerDelegate for Delegate { ), picker::PickerAction::separator(), picker::PickerAction::button("Open File", menu::Confirm.boxed_clone()), - picker::PickerAction::button("To project search", super::ToProjectSearch.boxed_clone()), + picker::PickerAction::button("Open as Tab", super::ToProjectSearch.boxed_clone()), ] } diff --git a/crates/settings_profile_selector/src/settings_profile_selector.rs b/crates/settings_profile_selector/src/settings_profile_selector.rs index 59dc0e4b505..873f16785fb 100644 --- a/crates/settings_profile_selector/src/settings_profile_selector.rs +++ b/crates/settings_profile_selector/src/settings_profile_selector.rs @@ -52,13 +52,8 @@ impl SettingsProfileSelector { window: &mut Window, cx: &mut Context, ) -> Self { - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(rems(22.)) - .minimum_results_width(rems(22.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = + cx.new(|cx| Picker::uniform_list(delegate, window, cx).initial_width(rems(22.))); Self { picker } } } diff --git a/crates/settings_ui/src/components/font_picker.rs b/crates/settings_ui/src/components/font_picker.rs index e8825b6c599..adeb7b048a5 100644 --- a/crates/settings_ui/src/components/font_picker.rs +++ b/crates/settings_ui/src/components/font_picker.rs @@ -180,8 +180,7 @@ pub fn font_picker( Picker::uniform_list(delegate, window, cx) .show_scrollbar(true) - .minimum_results_width(rems_from_px(210.)) - .height(rems(18.)) - .no_vertical_padding() - .modal(false) + .initial_width(rems_from_px(210.)) + .max_height(rems(18.)) + .popover() } diff --git a/crates/settings_ui/src/components/icon_theme_picker.rs b/crates/settings_ui/src/components/icon_theme_picker.rs index 903a57b5a51..3fd6ef12faa 100644 --- a/crates/settings_ui/src/components/icon_theme_picker.rs +++ b/crates/settings_ui/src/components/icon_theme_picker.rs @@ -193,8 +193,7 @@ pub fn icon_theme_picker( Picker::uniform_list(delegate, window, cx) .show_scrollbar(true) - .minimum_results_width(rems_from_px(210.)) - .height(rems(18.)) - .no_vertical_padding() - .modal(false) + .initial_width(rems_from_px(210.)) + .max_height(rems(18.)) + .popover() } diff --git a/crates/settings_ui/src/components/ollama_model_picker.rs b/crates/settings_ui/src/components/ollama_model_picker.rs index d6f0e7249c8..46cb38f0fec 100644 --- a/crates/settings_ui/src/components/ollama_model_picker.rs +++ b/crates/settings_ui/src/components/ollama_model_picker.rs @@ -204,10 +204,9 @@ pub fn render_ollama_model_picker( Picker::uniform_list(delegate, window, cx) .show_scrollbar(true) - .minimum_results_width(rems_from_px(210.)) - .height(rems(18.)) - .no_vertical_padding() - .modal(false) + .initial_width(rems_from_px(210.)) + .max_height(rems(18.)) + .popover() })) }) .anchor(gpui::Anchor::TopLeft) diff --git a/crates/settings_ui/src/components/theme_picker.rs b/crates/settings_ui/src/components/theme_picker.rs index 0980de1e4ee..dc8e3f7ac74 100644 --- a/crates/settings_ui/src/components/theme_picker.rs +++ b/crates/settings_ui/src/components/theme_picker.rs @@ -178,8 +178,7 @@ pub fn theme_picker( Picker::uniform_list(delegate, window, cx) .show_scrollbar(true) - .minimum_results_width(rems_from_px(210.)) - .height(rems(18.)) - .no_vertical_padding() - .modal(false) + .initial_width(rems_from_px(210.)) + .max_height(rems(18.)) + .popover() } diff --git a/crates/snippets_ui/src/snippets_ui.rs b/crates/snippets_ui/src/snippets_ui.rs index 9fc0a3fcb37..22d77bb71f1 100644 --- a/crates/snippets_ui/src/snippets_ui.rs +++ b/crates/snippets_ui/src/snippets_ui.rs @@ -111,13 +111,7 @@ impl ScopeSelector { let delegate = ScopeSelectorDelegate::new(workspace, cx.entity().downgrade(), language_registry); - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx)); Self { picker } } diff --git a/crates/tab_switcher/src/tab_switcher.rs b/crates/tab_switcher/src/tab_switcher.rs index b66fbf0ad5c..2f79ad53014 100644 --- a/crates/tab_switcher/src/tab_switcher.rs +++ b/crates/tab_switcher/src/tab_switcher.rs @@ -177,7 +177,6 @@ impl TabSwitcher { Picker::nonsearchable_list(delegate, window, cx) } .initial_width(rems(PANEL_WIDTH_REMS)) - .minimum_results_width(rems(PANEL_WIDTH_REMS)) }), init_modifiers, } diff --git a/crates/tasks_ui/src/modal.rs b/crates/tasks_ui/src/modal.rs index a96e9db92ca..39a3baf842e 100644 --- a/crates/tasks_ui/src/modal.rs +++ b/crates/tasks_ui/src/modal.rs @@ -6,7 +6,7 @@ use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ Action, AnyElement, App, AppContext as _, Context, DismissEvent, Entity, EventEmitter, Focusable, InteractiveElement, ParentElement, Render, Styled, Subscription, Task, WeakEntity, - Window, rems, + Window, }; use itertools::Itertools; use picker::{Picker, PickerDelegate, highlighted_match_with_paths::HighlightedMatch}; @@ -148,11 +148,7 @@ impl TasksModal { window, cx, ) - .modal(is_modal) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - .height(rems(24.)) - .no_vertical_padding() + .when(!is_modal, |picker| picker.embedded()) }); let mut _subscriptions = [ cx.subscribe(&picker, |_, _, _: &DismissEvent, cx| { diff --git a/crates/theme_selector/src/icon_theme_selector.rs b/crates/theme_selector/src/icon_theme_selector.rs index 5318a13df73..f677ce27da7 100644 --- a/crates/theme_selector/src/icon_theme_selector.rs +++ b/crates/theme_selector/src/icon_theme_selector.rs @@ -45,11 +45,7 @@ impl IconThemeSelector { window: &mut Window, cx: &mut Context, ) -> Self { - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx)); Self { picker } } } diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 78a8d0b3b67..a245e95529b 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -119,11 +119,7 @@ impl ThemeSelector { window: &mut Window, cx: &mut Context, ) -> Self { - let picker = cx.new(|cx| { - Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) - }); + let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx)); Self { picker } } } diff --git a/crates/toolchain_selector/src/toolchain_selector.rs b/crates/toolchain_selector/src/toolchain_selector.rs index 85b9bbdadc8..a85b722a6d5 100644 --- a/crates/toolchain_selector/src/toolchain_selector.rs +++ b/crates/toolchain_selector/src/toolchain_selector.rs @@ -112,9 +112,7 @@ impl AddToolchainState { let (lister, rx) = Self::create_path_browser_delegate(project.clone(), cx); let path_style = project.read(cx).path_style(cx); let picker = cx.new(|cx| { - let picker = Picker::uniform_list(lister, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)); + let picker = Picker::uniform_list(lister, window, cx); let mut worktree_root = worktree_root_path.to_string_lossy().into_owned(); worktree_root.push_str(path_style.primary_separator()); picker.set_query(&worktree_root, window, cx); @@ -234,9 +232,7 @@ impl AddToolchainState { let (delegate, rx) = Self::create_path_browser_delegate(this.project.clone(), cx); picker.update(cx, |picker, cx| { - *picker = Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)); + *picker = Picker::uniform_list(delegate, window, cx); picker.set_query(path.to_string_lossy().as_ref(), window, cx); }); *input_state = Self::wait_for_path(rx, window, cx); @@ -688,8 +684,6 @@ impl ToolchainSelector { cx, ); Picker::uniform_list(delegate, window, cx) - .initial_width(rems(34.)) - .minimum_results_width(rems(34.)) }); let picker_focus_handle = picker.focus_handle(cx); picker.update(cx, |picker, _| { diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index a5aa8ba4b59..c76579311ec 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -1433,9 +1433,7 @@ impl RegistersView { matches, }; - Picker::nonsearchable_uniform_list(delegate, window, cx) - .initial_width(rems(36.)) - .modal(true) + Picker::nonsearchable_uniform_list(delegate, window, cx).initial_width(rems(36.)) } } @@ -1800,9 +1798,7 @@ impl MarksView { matches, workspace, }; - Picker::nonsearchable_uniform_list(delegate, window, cx) - .initial_width(rems(36.)) - .modal(true) + Picker::nonsearchable_uniform_list(delegate, window, cx).initial_width(rems(36.)) } }