Revert UI revamp from PR #27#28
Conversation
This reverts commit 8ac5818.
There was a problem hiding this comment.
Code Review
This pull request implements a significant UI overhaul, simplifying the design system by updating CSS variables, removing complex gradients, and standardizing component styles across the web and UI packages. Notable changes include refactoring the tasks view into a table-based layout and streamlining the authentication view. Feedback identifies a redundant CSS selector in the global stylesheet and highlights accessibility regressions caused by the removal of focus indicators on input and textarea components.
| .calendar-shell { | ||
| padding: 18px; | ||
| padding: 14px; | ||
| } | ||
|
|
||
| .calendar-shell { | ||
| display: grid; | ||
| grid-template-columns: minmax(0, 1fr) 320px; | ||
| min-height: 0; | ||
| overflow: hidden; | ||
| background: rgba(255, 255, 255, 0.78); | ||
| } |
There was a problem hiding this comment.
The .calendar-shell selector is defined twice consecutively due to the revert. These blocks should be merged into a single rule to improve maintainability and reduce redundancy in the stylesheet.
.calendar-shell {
padding: 14px;
display: grid;
grid-template-columns: minmax(0, 1fr) 320px;
min-height: 0;
overflow: hidden;
}| ref={ref} | ||
| className={cn( | ||
| "flex h-11 w-full rounded-[14px] border border-input bg-[color:var(--surface-1)] px-4 py-2 text-[0.92rem] text-[color:var(--text)] shadow-[inset_0_1px_0_rgba(255,255,255,0.75)] outline-none transition-[border-color,box-shadow,background-color] duration-150 ease-[var(--ease-out)] placeholder:text-[color:var(--text-subtle)] focus:border-[color:var(--line-emphasis)] focus:bg-white focus:shadow-[0_0_0_4px_var(--ring)] disabled:cursor-not-allowed disabled:opacity-50", | ||
| "flex h-10 w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm text-[var(--text)] shadow-none outline-none placeholder:text-[var(--muted)] focus-visible:ring-0 disabled:cursor-not-allowed disabled:opacity-50", |
| ref={ref} | ||
| className={cn( | ||
| "flex min-h-[110px] w-full rounded-[16px] border border-input bg-[color:var(--surface-1)] px-4 py-3 text-[0.92rem] text-[color:var(--text)] shadow-[inset_0_1px_0_rgba(255,255,255,0.75)] outline-none transition-[border-color,box-shadow,background-color] duration-150 ease-[var(--ease-out)] placeholder:text-[color:var(--text-subtle)] focus:border-[color:var(--line-emphasis)] focus:bg-white focus:shadow-[0_0_0_4px_rgba(37,99,235,0.12)] disabled:cursor-not-allowed disabled:opacity-50", | ||
| "flex min-h-[80px] w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm text-[var(--text)] shadow-none outline-none placeholder:text-[var(--muted)] focus-visible:ring-0 disabled:cursor-not-allowed disabled:opacity-50", |
There was a problem hiding this comment.
Pull request overview
This PR reverts the UI revamp introduced in PR #27, restoring the pre-revamp visual system and screen presentation across auth, tasks, mail, calendar, and shared UI components.
Changes:
- Roll back global design tokens/layout and component styling to the pre-revamp look.
- Revert Auth and Tasks screens from the revamped layouts back to simpler shells.
- Remove revamp-specific dialog/button/input styling and related visual effects.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
packages/ui/src/textarea.tsx |
Reverts textarea styling to the pre-revamp look. |
packages/ui/src/input.tsx |
Reverts input styling to the pre-revamp look. |
packages/ui/src/dialog.tsx |
Reverts dialog overlay/content styling. |
packages/ui/src/button.tsx |
Reverts button base/variants/sizes styling. |
packages/ui/src/app-rail.tsx |
Reverts app rail sizing and colors. |
packages/features/src/tasks/tasks-view.tsx |
Reverts Tasks screen layout and presentation (table-based view). |
packages/features/src/auth/auth-view.tsx |
Reverts Auth screen layout and copy to pre-revamp presentation. |
apps/web/app/globals.css |
Restores pre-revamp global tokens, layout sizing, and surface styling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const BUTTON_BASE = | ||
| "inline-flex items-center justify-center gap-2 rounded-[14px] text-[0.82rem] font-semibold tracking-[-0.01em] transition-[transform,background-color,border-color,color,box-shadow,opacity] duration-150 ease-[var(--ease-out)] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[color:var(--ring)] focus-visible:ring-offset-2 focus-visible:ring-offset-[color:var(--surface-0)] active:scale-[0.98] disabled:pointer-events-none disabled:opacity-50"; | ||
| "inline-flex items-center justify-center rounded-full text-[0.82rem] font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#94a3b8] focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50"; |
There was a problem hiding this comment.
BUTTON_BASE no longer includes any gap-* utility. This breaks existing usages that render an icon + label inside a single <Button> (e.g., the calendar "Delete event" button), causing the icon and text to touch. Restore a default gap (or otherwise ensure icon+text layouts have spacing by default).
| "inline-flex items-center justify-center rounded-full text-[0.82rem] font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#94a3b8] focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50"; | |
| "inline-flex items-center justify-center gap-1.5 rounded-full text-[0.82rem] font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#94a3b8] focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50"; |
| <select | ||
| className="h-11 rounded-[14px] border border-[color:var(--line)] bg-[color:var(--surface-1)] px-4 text-[0.9rem] text-[color:var(--text)] outline-none transition-[border-color,box-shadow,background-color] duration-150 ease-[var(--ease-out)] focus:border-[color:var(--line-emphasis)] focus:bg-white focus:shadow-[0_0_0_4px_var(--ring)]" | ||
| className="h-10 rounded-md border border-[var(--line)] bg-white px-3 text-sm text-[var(--text)] outline-none" | ||
| value={statusFilter} | ||
| onChange={(event) => | ||
| setStatusFilter(event.target.value as StatusFilter) |
There was a problem hiding this comment.
This <select> uses outline-none but does not add any focus/focus-visible styling, so it can lose the visible focus indicator for keyboard users. Add a visible focus style (ring or border change), consistent with other form controls.
| <select | ||
| className="h-11 rounded-[14px] border border-[color:var(--line)] bg-[color:var(--surface-1)] px-4 text-[0.9rem] text-[color:var(--text)] outline-none transition-[border-color,box-shadow,background-color] duration-150 ease-[var(--ease-out)] focus:border-[color:var(--line-emphasis)] focus:bg-white focus:shadow-[0_0_0_4px_var(--ring)]" | ||
| className="h-10 rounded-md border border-[var(--line)] bg-white px-3 text-sm text-[var(--text)] outline-none" | ||
| value={categoryFilter} | ||
| onChange={(event) => setCategoryFilter(event.target.value)} | ||
| aria-label="Filter by category" |
There was a problem hiding this comment.
This <select> also uses outline-none without any replacement focus/focus-visible styling. Add a visible focus indicator (ring/border) for keyboard accessibility.
| } | ||
|
|
||
| .calendar-shell { |
There was a problem hiding this comment.
.calendar-shell is declared twice back-to-back. This is redundant and makes it easier to accidentally diverge the definitions later. Consider merging these into a single rule block.
| } | |
| .calendar-shell { |
| ref={ref} | ||
| className={cn( | ||
| "flex h-11 w-full rounded-[14px] border border-input bg-[color:var(--surface-1)] px-4 py-2 text-[0.92rem] text-[color:var(--text)] shadow-[inset_0_1px_0_rgba(255,255,255,0.75)] outline-none transition-[border-color,box-shadow,background-color] duration-150 ease-[var(--ease-out)] placeholder:text-[color:var(--text-subtle)] focus:border-[color:var(--line-emphasis)] focus:bg-white focus:shadow-[0_0_0_4px_var(--ring)] disabled:cursor-not-allowed disabled:opacity-50", | ||
| "flex h-10 w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm text-[var(--text)] shadow-none outline-none placeholder:text-[var(--muted)] focus-visible:ring-0 disabled:cursor-not-allowed disabled:opacity-50", |
There was a problem hiding this comment.
Input removes the native focus outline (outline-none) and explicitly disables the Tailwind focus ring (focus-visible:ring-0) without adding any alternative focus styling. This makes keyboard focus hard to see. Add a visible focus style (e.g., focus-visible ring or border change) or avoid removing the outline.
| "flex h-10 w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm text-[var(--text)] shadow-none outline-none placeholder:text-[var(--muted)] focus-visible:ring-0 disabled:cursor-not-allowed disabled:opacity-50", | |
| "flex h-10 w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm text-[var(--text)] shadow-none outline-none placeholder:text-[var(--muted)] focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50", |
| ref={ref} | ||
| className={cn( | ||
| "flex min-h-[110px] w-full rounded-[16px] border border-input bg-[color:var(--surface-1)] px-4 py-3 text-[0.92rem] text-[color:var(--text)] shadow-[inset_0_1px_0_rgba(255,255,255,0.75)] outline-none transition-[border-color,box-shadow,background-color] duration-150 ease-[var(--ease-out)] placeholder:text-[color:var(--text-subtle)] focus:border-[color:var(--line-emphasis)] focus:bg-white focus:shadow-[0_0_0_4px_rgba(37,99,235,0.12)] disabled:cursor-not-allowed disabled:opacity-50", | ||
| "flex min-h-[80px] w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm text-[var(--text)] shadow-none outline-none placeholder:text-[var(--muted)] focus-visible:ring-0 disabled:cursor-not-allowed disabled:opacity-50", |
There was a problem hiding this comment.
Textarea removes the native focus outline (outline-none) and disables the focus ring (focus-visible:ring-0) without providing an alternative focus indicator. Add a visible focus style (ring/border) or keep the default outline for keyboard accessibility.
| "flex min-h-[80px] w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm text-[var(--text)] shadow-none outline-none placeholder:text-[var(--muted)] focus-visible:ring-0 disabled:cursor-not-allowed disabled:opacity-50", | |
| "flex min-h-[80px] w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm text-[var(--text)] shadow-none outline-none placeholder:text-[var(--muted)] focus-visible:border-ring focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50", |
|
|
||
| const BUTTON_BASE = | ||
| "inline-flex items-center justify-center gap-2 rounded-[14px] text-[0.82rem] font-semibold tracking-[-0.01em] transition-[transform,background-color,border-color,color,box-shadow,opacity] duration-150 ease-[var(--ease-out)] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[color:var(--ring)] focus-visible:ring-offset-2 focus-visible:ring-offset-[color:var(--surface-0)] active:scale-[0.98] disabled:pointer-events-none disabled:opacity-50"; | ||
| "inline-flex items-center justify-center rounded-full text-[0.82rem] font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#94a3b8] focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50"; |
There was a problem hiding this comment.
🟡 Removing gap-2 from BUTTON_BASE breaks icon+text spacing in existing callers
The old BUTTON_BASE included gap-2 to space children (icons alongside text). The new version removes all gap utilities. This causes a visual regression in buttons that render both an icon and text as direct children without providing their own gap class. Specifically, the Delete event button at packages/features/src/calendar/calendar-dialogs.tsx:231-232 renders <Trash2 size={14} />Delete event inside a <Button> whose className (border-[#fecdd3] text-[#be123c] hover:bg-[#fff1f2]) does not include any gap-* override. The icon and text will be rendered adjacent with zero spacing.
| "inline-flex items-center justify-center rounded-full text-[0.82rem] font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#94a3b8] focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50"; | |
| "inline-flex items-center justify-center gap-2 rounded-full text-[0.82rem] font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#94a3b8] focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50"; |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
8ac5818from PR Revamp InboxOS UI system and core screens #27Testing
Notes