migrate overlays and feedback
This commit is contained in:
+95
-15
@@ -21,7 +21,11 @@ repository at `main`, and this workspace's `Cargo.lock` (zed at `4b47ceb`,
|
||||
- **Phase 2: landed.** `input/` runs on base's editing engine. 6,715 lines of
|
||||
engine and history are gone and 306 are written, taking `crates/ui/src/input`
|
||||
from 6,573 lines to 321. Six call sites changed, all named in phase 2 below.
|
||||
- **Phases 3-5: not started.**
|
||||
- **Phase 3: landed.** `tooltip`, `popover`, `modal`, and `notification` run on base's
|
||||
overlay and feedback primitives. Those four modules are 1,434 lines where they
|
||||
were 1,592, and nothing outside `crates/ui` changed. The behavioural differences
|
||||
are named in phase 3 below; the largest is that base's toast stack replaces the
|
||||
fork's notification list.
|
||||
- One pre-existing, unrelated breakage was found; see
|
||||
[A pre-existing wasm blocker](#a-pre-existing-wasm-blocker).
|
||||
|
||||
@@ -109,6 +113,9 @@ dragging, both of which are projected.
|
||||
|
||||
## What each module becomes
|
||||
|
||||
LOC is the count before the work; a module whose phase has landed reads
|
||||
`before → after`.
|
||||
|
||||
| `ui` module | LOC | Plan | `gpui-base` counterpart |
|
||||
| --- | --- | --- | --- |
|
||||
| `input/` (input, clear_button) | 6,573 | Replace; keep `ui::input::{Input, InputEvent, InputState}` as the import path. 321 lines remain, and the engine paints itself through `InputEditorStyle` | `InputState`/`TextareaState` (`InputBaseState` in two modes) plus the `InputBase` frame |
|
||||
@@ -116,10 +123,10 @@ dragging, both of which are projected.
|
||||
| `checkbox.rs` | 312 | Delete | `Checkbox` |
|
||||
| `scroll/` (scrollbar, scrollable, scrollable_mask) | 1,332 | Replace; keep the `ScrollableElement` and `Scrollbar` names | `Scrollbar`, `ScrollableMask` |
|
||||
| `resizable/` | 927 | Replace; base exports the same names (`h_resizable`, `v_resizable`, `resizable_panel`, `PANEL_MIN_SIZE`, `resize_handle`) | `Resizable` + `ResizeHandleRenderer` for the coop hairline |
|
||||
| `modal.rs` | 540 | Port onto base parts; keep `Modal`, `ModalButtonProps`, and `window.open_modal` | `Dialog`, `AlertDialog` |
|
||||
| `notification.rs` | 584 | Port; keep `Notification`, `NotificationKind`, and `window.push_notification` | `Toast`, `ToastManager`, `ToastStack` |
|
||||
| `popover.rs` | 432 | Replace with a coop-styled wrapper | `Popover`, `Popup`, `Positioner` |
|
||||
| `tooltip.rs` | 36 | Replace with a coop-styled wrapper | `Tooltip` |
|
||||
| `modal.rs` | 540 → 500 | Port onto base parts; `Modal`, `ModalButtonProps`, and `window.open_modal` unchanged. `Root` still owns the stack | `Dialog` — focus trap, Escape/Enter/backdrop dispatch, layer priority, deferred host |
|
||||
| `notification.rs` | 584 → 663 | Port; `Notification`, `NotificationKind`, and `window.push_notification` unchanged | `ToastManager` (storage, ids, timers, exit), `ToastStack` (geometry, motion), `Toast` (`Role::Alert`) |
|
||||
| `popover.rs` | 432 → 234 | Coop's builder over base's element; `PopoverState` is base's, re-exported | `Popover`, `Popup`, `Positioner` |
|
||||
| `tooltip.rs` | 36 → 37 | Coop's view rooted at base's element | `Tooltip` (`Role::Tooltip`) |
|
||||
| `button.rs` | 626 | Skin: base behavior plus coop's existing variant tables | `Button`, `StateStyle` |
|
||||
| `switch.rs` | 287 | Skin | `Switch`, `SwitchTrack`, `SwitchThumb` |
|
||||
| `avatar.rs` | 141 | Skin | `Avatar`, `AvatarImage`, `AvatarFallback` |
|
||||
@@ -310,18 +317,86 @@ Surfaces to re-verify by hand: the chat composer (auto-grow, Enter to send, IME)
|
||||
profile bio, the subject line, the settings dialog, the relay and messaging lists, the
|
||||
import/restore/backup dialogs, and the sidebar search field.
|
||||
|
||||
### Phase 3 — overlays and feedback
|
||||
### Phase 3 — overlays and feedback — landed
|
||||
|
||||
`popover` becomes a wrapper over base `Popover`; `modal` composes base `Dialog` and
|
||||
`AlertDialog` while keeping the `Modal` API and `window.open_modal`; `notification`
|
||||
moves onto `Toast`/`ToastManager` (base owns the stack, timers, and motion; coop owns
|
||||
the visual and the placement from `theme.notification`); `tooltip` becomes a wrapper
|
||||
over base `Tooltip`. `Root` and `window_ext` keep their public API and host the new
|
||||
layers. No call site changes.
|
||||
All four modules keep their names, builders, and call sites. The four files go from
|
||||
1,592 lines to 1,434, and no file outside `crates/ui` changed.
|
||||
|
||||
| `ui` module | What stayed coop's | What is base's now |
|
||||
| --- | --- | --- |
|
||||
| `tooltip` | the whole look, `Tooltip::new(text, window, cx)` and the `Render` view | the element and `Role::Tooltip` |
|
||||
| `popover` | every builder, the content styling, the anchor | open lifecycle, dismissal, focus capture and restore, deferred registration, trigger measurement and anchor math |
|
||||
| `modal` | `Modal`, `ModalButtonProps`, `Root`'s stack, `window.open_modal`, the card, buttons, shadows and animations | focus trap, Escape/Enter/backdrop dispatch with a cancel veto, layer priority, the deferred host, `Role::Dialog` |
|
||||
| `notification` | `Notification`, `NotificationKind`, `window.push_notification`, the card and the placement from `theme.notification` | id-replacing storage, auto-hide and exit timers, stack geometry and motion, `Role::Alert` |
|
||||
|
||||
**`tooltip`.** The view and its `new` are unchanged; the styled box inside is
|
||||
`gpui_base::Tooltip` instead of a bare `div`. That is what carries the role. Base's
|
||||
window-level `TooltipOverlay` is deliberately not adopted — gpui's own `.tooltip()`
|
||||
layer already provides the delay and the placement, and taking the overlay would mean
|
||||
rewriting every `.tooltip(..)` call site onto `Popup` plus hover state.
|
||||
|
||||
**`popover`.** `PopoverState` is `gpui_base::PopoverState`, re-exported so
|
||||
`ui::popover::PopoverState` still resolves, and the hand-rolled `anchored`/`deferred`
|
||||
layer, `resolved_corner` and `render_popover` are gone — base's `Popup` measures the
|
||||
trigger, resolves the anchor and snaps to the window edge. The rest of the file is the
|
||||
fork's builder, unchanged, including `trigger_style`, which the fork already stored
|
||||
without ever reading. Two bindings changed hands: `popover::init` (escape → coop's
|
||||
`Cancel` in the `Popover` context) is deleted, because `gpui_base::init` binds
|
||||
escape/enter/space in that same context and coop's lone escape binding would have
|
||||
shadowed base's `Confirm` — the one that opens a popover from its trigger.
|
||||
|
||||
**`modal`.** `Modal` still assembles the card, title, close button, footer buttons,
|
||||
the two shadows and the `fade-in`/`slide-down` animations; `Root` still owns the stack,
|
||||
the focus restore, and the one-visible-overlay rule, now expressed as base's
|
||||
`layer(index, topmost)`. What changed underneath:
|
||||
|
||||
- Escape, Enter and the backdrop now run through base's `Dialog` decisions, so
|
||||
`on_cancel`/`on_ok` returning `false` vetoes all three. The fork honored the veto on
|
||||
the buttons and the backdrop but ignored it on Escape.
|
||||
- Enter on a modal that has a footer but no `on_ok` now calls `on_close` before closing;
|
||||
the fork closed silently. No caller combines the two, and `on_close` defaults to a
|
||||
no-op.
|
||||
- Tab is trapped inside the modal, and the dialog surface carries `Role::Dialog`.
|
||||
- `modal::init` (escape/enter in the `Modal` context) is deleted; base binds them in
|
||||
its own `Dialog` context, which the `Dialog` host installs when `keyboard` is on.
|
||||
- The dim does not move: coop's backdrop element keeps the `window_paddings` inset and
|
||||
the `view_size` that the fork used. Its hit area does move — base's host covers the
|
||||
whole viewport, so a click in the client-side-decoration shadow band now dismisses
|
||||
the modal instead of starting a window resize.
|
||||
|
||||
`AlertDialog` turned out to be unnecessary. Coop's `alert()` and `confirm()` select a
|
||||
button set, not an ARIA role, and they already opt out of backdrop dismissal, which is
|
||||
the whole of what `AlertDialog` adds over `Dialog`.
|
||||
|
||||
**`notification`.** `Notification` keeps its builder and its card. `closing: bool`
|
||||
becomes base's `ToastTransitionStatus`, `dismiss` now emits a `DismissRequest` the list
|
||||
turns into a `ToastManager::dismiss`, and the exit delay is base's 200 ms rather than
|
||||
the fork's fixed 150 ms. `NotificationList` holds
|
||||
`ToastManager<NotificationId, Entity<Notification>>` plus one `ToastStackState`; its
|
||||
`expanded` field and hover handler are gone, and a 50 ms lifecycle tick runs only while
|
||||
something is mounted. The stack is base's:
|
||||
|
||||
- It collapses to three layers with a 14 px peek and a 5% width step per layer, expands
|
||||
on hover or focus, and pauses auto-hide while expanded.
|
||||
- The newest notification sits nearest the window edge; the fork's list grew downwards
|
||||
with the oldest first.
|
||||
- Motion is `ToastMotion::default()`, base's shadcn/Sonner figures. Coop contributes the
|
||||
width the fork's card had, the placement and the margins from `theme.notification`.
|
||||
|
||||
That stack is the one visible change of the phase, and it is the one to judge by hand.
|
||||
If it is not wanted, the smaller step is to keep the list's own `v_flex` and use only
|
||||
`ToastManager` together with `Toast` — base separates the lifecycle from the geometry,
|
||||
so nothing else has to come back.
|
||||
|
||||
Surfaces to re-verify by hand: the settings dialog (its Escape and Enter paths), the
|
||||
import, restore and screening modals (a modal with a textarea, and one with
|
||||
`keyboard(false)`), the dropdown menus that ride the popover, and every
|
||||
`push_notification` site — sending an empty message, a failed upload with its retry
|
||||
action, and the device-approval notification that never auto-hides.
|
||||
|
||||
### Phase 4 — leaf controls, scroll, and resizable (one module per pull request)
|
||||
|
||||
Order: `tooltip`, `avatar`, `switch`, `button`, `scroll/`, `resizable/`. `button` is the
|
||||
Order: `avatar`, `switch`, `button`, `scroll/`, `resizable/`. `button` is the
|
||||
largest skin: the `ButtonVariants` and `ButtonCustomVariant` tables, the `compact`,
|
||||
`loading`, and `caret` builders, and the variant names stay as they are, with styling
|
||||
supplied through base's semantic-state styles. `scroll/` keeps the `ScrollableElement`
|
||||
@@ -345,7 +420,12 @@ menu positioning and dismissal on base `Popup`/`Positioner` is optional and late
|
||||
|
||||
There is no UI test suite to lean on, so each phase gets the same treatment:
|
||||
|
||||
- `cargo check` and `cargo build` at the workspace root.
|
||||
- `cargo check --workspace` and `cargo build` (default members build `desktop`).
|
||||
`cargo build --workspace` cannot link the web crate's host dylib: `coop_web` is
|
||||
`crate-type = ["cdylib", "rlib"]` and depends on `wasm-bindgen`, `web-sys`,
|
||||
`console_log` and `tracing-wasm` unconditionally, so its dylib is a wasm artifact.
|
||||
That is a property of the manifest rather than of any migrated crate —
|
||||
`cargo check -p coop_web` passes, and the desktop binary links the same crates.
|
||||
- `cargo check -p theme -p ui --target wasm32-unknown-unknown`. The web target cannot
|
||||
be checked end to end until the pre-existing blocker below is fixed, so the migrated
|
||||
crates are checked directly.
|
||||
@@ -402,7 +482,7 @@ compile for `wasm32-unknown-unknown`".
|
||||
| 1 | Phase 0: `gpui` moves to the `gpui-pre` package, `gpui_tokio` vendored | root `Cargo.toml`, `Cargo.lock`, `web/Cargo.toml`, new `crates/gpui_tokio`; `crates/state` needed no edit | landed |
|
||||
| 2 | Phase 1: base wiring, `sync_base`, deletions | `crates/theme` | landed |
|
||||
| 3 | Phase 2: input, plus `history.rs` and the `ropey`/`sum_tree`/`lsp-types`/`regex`/`unicode-segmentation`/`tree-sitter` pruning | `crates/workspace`, `crates/chat_ui` (six call sites); no manifest outside `crates/ui` | landed |
|
||||
| 4 | Phase 3: popover, modal, notification, tooltip | none | not started |
|
||||
| 4 | Phase 3: popover, modal, notification, tooltip | none | landed |
|
||||
| 5–10 | Phase 4: one leaf module each | none | not started |
|
||||
| later | Phase 5: dock, as its own plan | `crates/workspace`, `crates/chat_ui` | not started |
|
||||
|
||||
|
||||
Reference in New Issue
Block a user