Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 24 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/ui/components/form/inputs/TextField.tsx">
<violation number="1" location="packages/ui/components/form/inputs/TextField.tsx:261">
P2: Hardcoded `text-gray-500` breaks the design system pattern. The codebase uses semantic color tokens (`text-default`, `text-muted`, `text-subtle`). Use `text-muted` for hint text to maintain theming consistency.</violation>
<violation number="2" location="packages/ui/components/form/inputs/TextField.tsx:262">
P2: Hardcoded `text-gray-500` breaks the design system pattern. Use a semantic color token like `text-muted` to match the parent div and maintain theming consistency.</violation>
</file>
<file name="apps/web/modules/embed/components/Embed.tsx">
<violation number="1" location="apps/web/modules/embed/components/Embed.tsx:1287">
P2: Hardcoded text "Embed theme" should be localized using `t()` function for consistency with other labels in this component (e.g., `t("layout")`, `t("duration")`). Consider adding a translation key like `"embed_theme"` and using `{t("embed_theme")}`.</violation>
</file>
<file name="apps/web/modules/ee/organizations/profile.tsx">
<violation number="1" location="apps/web/modules/ee/organizations/profile.tsx:386">
P2: Using `document.querySelector` with programmatic click on a hidden element is fragile and non-idiomatic in React. Consider using a ref or lifting the dialog state up to control visibility directly. If the BannerUploader exposes a way to control its dialog state, use that instead of DOM manipulation. The current approach creates tight coupling to the `data-testid` attribute and silently fails if the element isn't found.</violation>
</file>
<file name="apps/web/modules/event-types/views/event-types-listing-view.tsx">
<violation number="1" location="apps/web/modules/event-types/views/event-types-listing-view.tsx:221">
P2: Hardcoded "(hidden)" text violates localization requirements. Use `t("hidden")` instead of the literal string to ensure proper internationalization.</violation>
</file>
<file name="apps/web/modules/ee/teams/components/createButton/CreateButton.tsx">
<violation number="1" location="apps/web/modules/ee/teams/components/createButton/CreateButton.tsx:99">
P2: The chevron-down icon is misleading here. This button branch (when `!hasTeams && !platform`) doesn't provide dropdown functionality - it only uses `options[0]`. The `EndIcon` suggests users can select from multiple options, but clicking won't show a dropdown. Consider removing this `EndIcon` from the non-dropdown button, or restructuring to show a dropdown when `hasMultipleOptions` is true regardless of team/platform status.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {embedParams.embedTabName !== EmbedTabName.ATOM_REACT && ( | ||
| <Label className="mb-6"> | ||
| <div className="mb-2">EmbedTheme</div> | ||
| <div className="mb-2">Embed theme</div> |
There was a problem hiding this comment.
P2: Hardcoded text "Embed theme" should be localized using t() function for consistency with other labels in this component (e.g., t("layout"), t("duration")). Consider adding a translation key like "embed_theme" and using {t("embed_theme")}.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/modules/embed/components/Embed.tsx, line 1287:
<comment>Hardcoded text "Embed theme" should be localized using `t()` function for consistency with other labels in this component (e.g., `t("layout")`, `t("duration")`). Consider adding a translation key like `"embed_theme"` and using `{t("embed_theme")}`.</comment>
<file context>
@@ -1126,13 +1275,16 @@ const EmbedTypeCodeAndPreviewDialogContent = ({
{embedParams.embedTabName !== EmbedTabName.ATOM_REACT && (
<Label className="mb-6">
- <div className="mb-2">EmbedTheme</div>
+ <div className="mb-2">Embed theme</div>
<Select
className="w-full"
</file context>
| StartIcon="upload" | ||
| onClick={() => { | ||
| // Trigger the BannerUploader dialog | ||
| const triggerButton = document.querySelector( |
There was a problem hiding this comment.
P2: Using document.querySelector with programmatic click on a hidden element is fragile and non-idiomatic in React. Consider using a ref or lifting the dialog state up to control visibility directly. If the BannerUploader exposes a way to control its dialog state, use that instead of DOM manipulation. The current approach creates tight coupling to the data-testid attribute and silently fails if the element isn't found.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/modules/ee/organizations/profile.tsx, line 386:
<comment>Using `document.querySelector` with programmatic click on a hidden element is fragile and non-idiomatic in React. Consider using a ref or lifting the dialog state up to control visibility directly. If the BannerUploader exposes a way to control its dialog state, use that instead of DOM manipulation. The current approach creates tight coupling to the `data-testid` attribute and silently fails if the element isn't found.</comment>
<file context>
@@ -239,119 +257,171 @@ const OrgProfileForm = ({ defaultValues }: { defaultValues: FormValues }) => {
+ StartIcon="upload"
+ onClick={() => {
+ // Trigger the BannerUploader dialog
+ const triggerButton = document.querySelector(
+ '[data-testid="open-upload-banner-dialog"]'
+ ) as HTMLButtonElement;
</file context>
apps/web/modules/event-types/views/event-types-listing-view.tsx
Outdated
Show resolved
Hide resolved
| } | ||
| data-testid="create-button" | ||
| StartIcon="plus" | ||
| EndIcon={hasMultipleOptions ? "chevron-down" : undefined} |
There was a problem hiding this comment.
P2: The chevron-down icon is misleading here. This button branch (when !hasTeams && !platform) doesn't provide dropdown functionality - it only uses options[0]. The EndIcon suggests users can select from multiple options, but clicking won't show a dropdown. Consider removing this EndIcon from the non-dropdown button, or restructuring to show a dropdown when hasMultipleOptions is true regardless of team/platform status.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/modules/ee/teams/components/createButton/CreateButton.tsx, line 99:
<comment>The chevron-down icon is misleading here. This button branch (when `!hasTeams && !platform`) doesn't provide dropdown functionality - it only uses `options[0]`. The `EndIcon` suggests users can select from multiple options, but clicking won't show a dropdown. Consider removing this `EndIcon` from the non-dropdown button, or restructuring to show a dropdown when `hasMultipleOptions` is true regardless of team/platform status.</comment>
<file context>
@@ -92,10 +96,15 @@ export function CreateButton(props: CreateBtnProps) {
}
data-testid="create-button"
StartIcon="plus"
+ EndIcon={hasMultipleOptions ? "chevron-down" : undefined}
loading={isPending}
variant={disableMobileButton ? "button" : "fab"}
</file context>
|
oh I think I messed that first one up when I fixed the merge conflict. will try and fix |
|
I think I fixed it @pedroccastro - can you confirm? I couldn't see the error again locally when testing after making the fix |
E2E results are ready! |
pedroccastro
left a comment
pedroccastro
left a comment
There was a problem hiding this comment.
Hey @pumfleet! Type checks and unit tests now passing!
Great work on the UX improvements, the search being inline will probably make it easier for users. The sentence case standardization makes it feel more polished.
Found a few things during local testing that might need a quick look:
1. TextField missing borders (without addons)
Inputs without addons (like the Title field in Add new event type dialog) are missing their borders. The URL field with the addon looks fine.
Likely cause: In TextField.tsx, the Input component (without addon) now receives border-0 bg-transparent in className, which overrides the border from inputStyles. This was probably copied from the input inside the addon wrapper (where border-0 is correct since the wrapper provides the border).
2. Hidden badge styling
The hidden indicator changed from a Badge component to plain text (hidden). A couple of things:
- Lost the
{t("hidden")}localization (now hardcoded English) - Uses
text-gray-400instead of design tokens
3. Mobile FAB showing two "+" icons
On mobile, when there are multiple team options, the FAB button shows two "+" icons instead of one.
Likely cause: The PR added EndIcon="chevron-down" to CreateButton. The core Button component has a quirk where both StartIcon and EndIcon render as "+" on mobile FAB variant.
Let me know if you'd like more details on any of these!
| export const FieldTypes = [ | ||
| { | ||
| label: "Short Text", | ||
| label: "Short text", |
There was a problem hiding this comment.
Shouldn't we use i18n for labels instead of harcoded strings?
There was a problem hiding this comment.
Ideally we should - but I wanted to keep this PR simple
| const _cookies = await cookies(); | ||
|
|
||
| const session = await getServerSession({ req: buildLegacyRequest(_headers, _cookies) }); | ||
| const session = await getServerSession({ |
There was a problem hiding this comment.
We would have to update the skeleton loader on this page as we moved the search input to the top
Screen.Recording.2026-01-10.at.6.24.10.PM.mov
FAB fixed Hidden translation string fixed Text fields fixed |
|
Hey @pumfleet! Thanks for addressing the previous feedback, the TextField borders and FAB icon issues are now fixed! 🎉 Did another round of local testing and the UX improvements are looking good! Create button size inconsistencyThe 
Also noticed there are 2 minor merge conflicts to resolve:
Both look straightforward (copy text difference and a Let me know if you'd like more details! |
- insights.e2e.ts: chart titles (14 strings) - event-types.e2e.ts: Organizer phone number location - EditLocationDialog.test.tsx: phone number labels
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/playwright/event-types.e2e.ts">
<violation number="1" location="apps/web/playwright/event-types.e2e.ts:226">
P1: Rule violated: **E2E Tests Best Practices**
Replace the new text locators that click the “Organizer phone number” option with resilient selectors (e.g., data-testid or role-based queries) to comply with the E2E Tests Best Practices rule.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
- Replace hardcoded text-gray-500 with text-muted in TextField.tsx hint section - Replace text locator with data-testid in E2E test for location select Co-Authored-By: unknown <>
- Use data-testid selectors for location options (more reliable than text) - Update field identifiers in routing-forms tests to match new labels - Fix Long text selector in manage-booking-questions test
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/playwright/manage-booking-questions.e2e.ts">
<violation number="1" location="apps/web/playwright/manage-booking-questions.e2e.ts:762">
P2: Rule violated: **E2E Tests Best Practices**
Replace the `text="Long text"` locator with a resilient selector (e.g., add/use a dedicated `data-testid` and query it via `getByTestId`) to comply with the E2E selector guideline and avoid brittle text matching.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
…s E2E test
Replace fragile text="Long text" locator with resilient
page.getByTestId("select-option-textarea") selector per E2E best practices.
Addresses Cubic AI review feedback (confidence 9/10).
Co-Authored-By: unknown <>
4 participants
This is a PR full of minor UI/UX fixes identified as part of the UX audit.
Summary by cubic
A sweep of UI/UX polish across Event Types, Workflows, Profiles, Embed, and Tables to make common actions clearer and faster. Highlights include an inline search with actions, a smarter Create button, improved uploads, and consistent copy.
UI Improvements
Bug Fixes
Written for commit 34de07d. Summary will update on new commits.
Updates since last revision
Addressed Cubic AI review feedback (confidence 9+ issues):
text-gray-500with semantictext-mutedcolor token for hint text styling to maintain design system consistency and theming supporttext="Organizer phone number"with resilientdata-testid="location-select-item-organizer_phone_number"selector per E2E best practicestext="Long text"with resilientpage.getByTestId("select-option-textarea")selector per E2E best practicesMandatory Tasks (DO NOT REMOVE)
How should this be tested?
yarn e2e apps/web/playwright/event-types.e2e.tsandyarn e2e apps/web/playwright/manage-booking-questions.e2e.tsto verify the tests pass with the new data-testid selectorsHuman Review Checklist
text-muted) renders correctly in both light and dark modeslocation-select-item-organizer_phone_numberdata-testid exists in LocationSelect componentselect-option-textareadata-testid exists in SelectField component (seepackages/ui/components/form/select/components.tsx:56)Link to Devin run: https://app.devin.ai/sessions/f52bbc36d7ba4e99bbaec5b295428264