-
Notifications
You must be signed in to change notification settings - Fork 9
Implement FAB + omnibox prototype #1970
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
Conversation
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.
6 issues found across 19 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.
<file name="packages/ui/src/v2/components/ct-chevron-button/ct-chevron-button.ts">
<violation number="1" location="packages/ui/src/v2/components/ct-chevron-button/ct-chevron-button.ts:142">
Please set this reusable chevron button’s type to "button" so it doesn’t accidentally submit parent forms when clicked.</violation>
</file>
<file name="packages/ui/src/v2/components/ct-fab/ct-fab.ts">
<violation number="1" location="packages/ui/src/v2/components/ct-fab/ct-fab.ts:54">
The backdrop mask is fixed to the bottom-right corner, so when `position` is set to other supported values the highlight is misaligned. Please parameterize the mask so it lines up with each position variant.</violation>
</file>
<file name="packages/ui/src/v2/components/ct-omnibox/ct-omnibox.ts">
<violation number="1" location="packages/ui/src/v2/components/ct-omnibox/ct-omnibox.ts:114">
ct-prompt-input already dispatches ct-send as a bubbling/composed event, so re-emitting it here without stopping the original causes container listeners to fire twice (e.g., the host sends a message twice). Either stopPropagation before re-dispatching or rely on the original event.</violation>
</file>
<file name="packages/patterns/omnibox-fab.tsx">
<violation number="1" location="packages/patterns/omnibox-fab.tsx:36">
Clicking the dismiss button stores the assistant message counter cell itself in `peekDismissedIndex`, so `count !== dismissedIdx` never flips false and the peek cannot stay dismissed. Capture the counter’s numeric value instead.</violation>
</file>
<file name="packages/ui/src/v2/components/ct-prompt-input/ct-prompt-input.ts">
<violation number="1" location="packages/ui/src/v2/components/ct-prompt-input/ct-prompt-input.ts:902">
When model is provided as a plain string (which the API promises to support), this handler only calls the cell controller, but the controller’s defaultSetValue does nothing for non-Cell values, so the component’s model property never reflects the newly chosen option. Please update the component to assign the new value itself when not bound to a Cell so consumers see the selection.</violation>
</file>
<file name="FAB_SPEC.md">
<violation number="1" location="FAB_SPEC.md:963">
Opening the history drawer marks notifications as seen, but the peek never refreshes, so it stays visible (and the FAB stays tall) after you close history. Call updateNotificationPeek() when setting peekDismissed to true so the UI hides immediately.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| backdrop-filter var(--ct-theme-animation-duration, 300ms) ease, | ||
| -webkit-backdrop-filter var(--ct-theme-animation-duration, 300ms) ease; | ||
| z-index: 998; | ||
| mask-image: radial-gradient( |
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.
The backdrop mask is fixed to the bottom-right corner, so when position is set to other supported values the highlight is misaligned. Please parameterize the mask so it lines up with each position variant.
Prompt for AI agents
Address the following comment on packages/ui/src/v2/components/ct-fab/ct-fab.ts at line 54:
<comment>The backdrop mask is fixed to the bottom-right corner, so when `position` is set to other supported values the highlight is misaligned. Please parameterize the mask so it lines up with each position variant.</comment>
<file context>
@@ -0,0 +1,397 @@
+ backdrop-filter var(--ct-theme-animation-duration, 300ms) ease,
+ -webkit-backdrop-filter var(--ct-theme-animation-duration, 300ms) ease;
+ z-index: 998;
+ mask-image: radial-gradient(
+ circle at bottom right,
+ rgba(0, 0, 0, 1) 0%,
</file context>
✅ Addressed in 584a7b2
packages/ui/src/v2/components/ct-prompt-input/ct-prompt-input.ts
Outdated
Show resolved
Hide resolved
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.
Reviewed changes from recent commits (found 2 issues).
2 issues found across 15 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/ui/src/v2/components/ct-fab/ct-fab.ts">
<violation number="1" location="packages/ui/src/v2/components/ct-fab/ct-fab.ts:414">
When previewMessage is supplied as a plain string, this branch never triggers, so showPreview stays false and the new preview notification never renders even though the property changed. Please also handle the non-Cell case so plain string updates display the notification.</violation>
</file>
<file name="packages/ui/src/v2/components/ct-prompt-input/ct-prompt-input.ts">
<violation number="1" location="packages/ui/src/v2/components/ct-prompt-input/ct-prompt-input.ts:441">
Guarding the model bind with a truthy check breaks plain string models—selecting an empty-string option never rebinds, so the select snaps back to the previous value. Please remove the truthy guard so empty string values can bind correctly.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
packages/ui/src/v2/components/ct-prompt-input/ct-prompt-input.ts
Outdated
Show resolved
Hide resolved
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.
Reviewed changes from recent commits (found 1 issue).
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/patterns/default-app.tsx">
<violation number="1" location="packages/patterns/default-app.tsx:125">
Capturing Cmd+O here prevents the browser’s standard “Open File” shortcut, so users lose access to a core browser feature.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| onct-keybind={spawnChatList()} | ||
| /> | ||
|
|
||
| <ct-keybind |
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.
Capturing Cmd+O here prevents the browser’s standard “Open File” shortcut, so users lose access to a core browser feature.
Prompt for AI agents
Address the following comment on packages/patterns/default-app.tsx at line 125:
<comment>Capturing Cmd+O here prevents the browser’s standard “Open File” shortcut, so users lose access to a core browser feature.</comment>
<file context>
@@ -116,6 +122,18 @@ export default recipe<CharmsListInput, CharmsListOutput>(
preventDefault
onct-keybind={spawnChatList()}
/>
+ <ct-keybind
+ code="KeyO"
+ meta
</file context>
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.
No issues found across 1 file
Summary by cubic
Introduces a morphing FAB that expands into an omnibox composer with a history toggle and notification peek, wired into the default app. Also fixes mention menu placement and smooths dismiss/escape interactions and toast behavior.
New Features
Bug Fixes