-
Notifications
You must be signed in to change notification settings - Fork 111
feat: enhance design #263
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
feat: enhance design #263
Conversation
|
@akash3444 is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughStyling and layout tweaks across three dashboard components: vertical centering of the registration checkbox, responsive/layout and panel positioning adjustments in the geo-map overview, and minor class/spacing plus dark-mode label color additions in the keyboard shortcuts UI. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile OverviewGreptile SummaryThis PR delivers three focused UI enhancements that improve the visual polish and user experience of the dashboard: Changes OverviewRegister Page - Fixed checkbox alignment issue where the terms acceptance checkbox was misaligned with its label text. Changed from Visitor Locations Card - Enhanced responsive layout with better mobile support:
Keyboard Shortcuts - Improved compact mode presentation:
Code QualityThe changes follow React best practices with proper use of Tailwind utilities and responsive breakpoints. Mobile responsiveness is thoughtfully handled across all changes. Two minor style suggestions were provided for the keyboard shortcuts component regarding Tailwind class ordering conventions and consistency with codebase patterns. ImpactThese are purely visual enhancements with no functional changes or breaking modifications. The improvements align with the stated goals in the PR description and screenshots demonstrate clear before/after improvements. Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant RegisterPage
participant GeoMapSection
participant KeyboardShortcuts
Note over User,KeyboardShortcuts: UI Enhancement Flow
User->>RegisterPage: View registration form
RegisterPage->>RegisterPage: Render checkbox with centered alignment
Note over RegisterPage: items-center aligns checkbox<br/>with label text vertically
User->>GeoMapSection: View visitor locations
GeoMapSection->>GeoMapSection: Apply responsive layout
Note over GeoMapSection: Mobile: aspect-video map<br/>Desktop: flex-row with sidebar
GeoMapSection->>GeoMapSection: Render top countries list
Note over GeoMapSection: Rounded borders + bg-muted<br/>for better visual hierarchy
User->>KeyboardShortcuts: View shortcuts in compact mode
KeyboardShortcuts->>KeyboardShortcuts: Apply dividers between groups
Note over KeyboardShortcuts: divide-y with improved<br/>dark mode contrast
KeyboardShortcuts->>KeyboardShortcuts: Render shortcut labels
Note over KeyboardShortcuts: Dark mode: text-foreground/70<br/>for better readability
|
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.
2 files reviewed, 2 comments
|
|
||
| return ( | ||
| <div className={compact ? "space-y-3" : "space-y-4"}> | ||
| <div className={compact ? "divide-y dark:divide-border divide-border/50" : "space-y-4"}> |
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 Tailwind class order is unconventional here. The standard pattern is to place base classes before dark mode variants for better readability and consistency with Tailwind conventions.
| <div className={compact ? "divide-y dark:divide-border divide-border/50" : "space-y-4"}> | |
| <div className={compact ? "divide-y divide-border/50 dark:divide-border" : "space-y-4"}> |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/components/ui/keyboard-shortcuts.tsx
Line: 68:68
Comment:
The Tailwind class order is unconventional here. The standard pattern is to place base classes before dark mode variants for better readability and consistency with Tailwind conventions.
```suggestion
<div className={compact ? "divide-y divide-border/50 dark:divide-border" : "space-y-4"}>
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| <div className={compact ? "divide-y dark:divide-border divide-border/50" : "space-y-4"}> | ||
| {groups.map((group) => ( | ||
| <div key={group.title}> | ||
| <div key={group.title} className={cn({"py-3 first:pt-0 last:pb-0": compact})}> |
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 object syntax for conditional classes with cn() is not the prevailing pattern in this codebase. Most conditional classes use ternary operators instead. Consider changing to match the codebase pattern.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/components/ui/keyboard-shortcuts.tsx
Line: 70:70
Comment:
The object syntax for conditional classes with cn() is not the prevailing pattern in this codebase. Most conditional classes use ternary operators instead. Consider changing to match the codebase pattern.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.|
Looks good, but I personally prefer the map to be native and full width, rather than floating with padding around it |
|
Should we have tabs inside that card, one for map and one for top countries? That way we can display map with full width |
|
or maybe we can have something like this... Screen.Recording.2026-01-10.at.8.18.28.PM.mov |
|
can you show a non zoomed in variant? |
|
yeah please no transparent variant, i meant zoomed out as in the entire page, how it looks |
|
yeah that looks great ^^ |
|
Let me push the new changes |
077db70 to
c5c9e3f
Compare
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx (1)
124-174: Hide the overlay on mobile or relocate the Top Countries list below the map.The absolutely positioned overlay (
absolute right-2 bottom-2 w-44) lacks responsive classes and will cover ~47% of mobile viewports (176px on 375px width), significantly obstructing the map on small screens. The parent container switches fromflex flex-col(mobile) tolg:flex-row(desktop), but the overlay remains fixed and always visible.Apply
hidden lg:blockto the overlay and move the Top Countries list below the map on mobile using a conditional render or toggle, or reduce the overlay's footprint on small screens.
🤖 Fix all issues with AI agents
In
@apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx:
- Line 124: The div using className "absolute right-2 bottom-2 z-1 w-44 shrink-0
rounded border border-t bg-muted" uses an invalid Tailwind class `z-1`; update
that class in the GeoMapSection component to a valid Tailwind z-index utility
(e.g., replace `z-1` with `z-10` or `z-0` depending on desired stacking) or, if
you truly need a custom value, add a `z` entry to the Tailwind config instead
and reference that custom class; ensure you update the className string in the
component where `z-1` appears (no other changes needed).
- Around line 114-116: Replace the inline style on the container element that
currently uses style={{ minHeight: 350 }} with Tailwind utility min-h-[350px];
locate the JSX element in geo-map-section.tsx (the div that wraps the map and
has the inline style) and remove the style prop, then add min-h-[350px] into
that element's className string (merging with existing classes like "relative
flex-1 max-lg:aspect-video lg:min-h-0 [&>div]:rounded-t-none [&>div]:border-0")
so there are no duplicate className props or leftover inline styles.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
**/*.{ts,tsx,js,jsx,vue}: Never block paste in<input>or<textarea>elements
Enter submits focused text input; in<textarea>, ⌘/Ctrl+Enter submits; Enter adds newline
Compatible with password managers and 2FA; allow pasting one-time codes
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{html,htm,tsx,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
**/*.{html,htm,tsx,jsx,vue}: Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for email/code/username inputs usingspellcheck="false"
Links are<a>or<Link>components for navigation; support Cmd/Ctrl/middle-click
Use politearia-livefor toasts and inline validation feedback
Autofocus on desktop when there's a single primary input; rarely on mobile to avoid layout shift
<title>tag must match current context
Use redundant status cues (not color-only); icon-only elements have text labels
Use accurate accessible names viaaria-label; mark decorative elements witharia-hidden; verify in Accessibility Tree
Icon-only buttons must have descriptivearia-label
Prefer native semantics (button,a,label,table) before ARIA
Use non-breaking spaces to glue terms:10 MB,⌘ + K,Vercel SDK
Preload only above-the-fold images; lazy-load the rest
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Trim values to handle text expansion and trailing spaces in form submissions
URL reflects state (deep-link filters/tabs/pagination/expanded panels); prefer libraries likenuqs
Back/Forward buttons restore scroll position
Delay first tooltip in a group; subsequent peers have no delay
Use locale-aware formatting for dates, times, numbers, and currency
Batch layout reads/writes; avoid unnecessary reflows/repaints
Virtualize large lists using libraries likevirtua
**/*.{ts,tsx,js,jsx}: Don't useaccessKeyattribute on any HTML element.
Don't setaria-hidden="true"on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Don't use distracting elements like<marquee>or<blink>.
Only use thescopeprop on<th>elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assigntabIndexto non-interactive HTML elements.
Don't use positive integers fortabIndexproperty.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include atitleelement for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
AssigntabIndexto non-interactive HTML elements witharia-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include atypeattribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden).
Always include...
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{css,scss,sass,less,html,htm,tsx,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
**/*.{css,scss,sass,less,html,htm,tsx,jsx,vue}: No dead zones on checkboxes/radios; label and control share one generous hit target
Usescroll-margin-topon headings for anchored links; include 'Skip to content' link; use hierarchical<h1>–<h6>
Prevent Cumulative Layout Shift (CLS) from images by using explicit dimensions or reserved space
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{ts,tsx,js,jsx,css,scss,sass,less}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
**/*.{ts,tsx,js,jsx,css,scss,sass,less}: During drag operations, disable text selection and setinerton dragged element and containers
Animations must be interruptible and input-driven; avoid autoplay
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{tsx,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
Don't ship the schema—visuals may omit labels but accessible names must still exist
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
Prefer uncontrolled inputs; make controlled loops cheap (optimize keystroke cost)
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,jsx}: Use semantic elements instead of role attributes in JSX.
Don't use unnecessary fragments.
Don't pass children as props.
Don't use the return value of React.render.
Make sure all dependencies are correctly specified in React hooks.
Make sure all React hooks are called from the top level of component functions.
Don't forget key props in iterators and collection literals.
Don't destructure props inside JSX components in Solid projects.
Don't define React components inside other components.
Don't use event handlers on non-interactive elements.
Don't assign to React component props.
Don't use bothchildrenanddangerouslySetInnerHTMLprops on the same element.
Don't use dangerous JSX props.
Don't use Array index in keys.
Don't insert comments as text nodes.
Don't assign JSX properties multiple times.
Don't add extra closing tags for components without children.
Use<>...</>instead of<Fragment>...</Fragment>.
Watch out for possible "wrong" semicolons inside JSX elements.
Make sure void (self-closing) elements don't have children.
Don't usetarget="_blank"withoutrel="noopener".
Don't use<img>elements in Next.js projects.
Don't use<head>elements in Next.js projects.
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't return a value from a function with the return type 'void'.
Don't use the TypeScript directive @ts-ignore.
Don't use TypeScript enums.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.
**/*.{ts,tsx}: Do NOT use types 'any', 'unknown' or 'never'. Use proper explicit types
Suffix functions with 'Action' in types, like 'type Test = { testAction }'
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
!(**/pages/_document.{ts,tsx,jsx})**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{ts,tsx,html,css}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuildfor bundling
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-MUST-DO.mdc)
**/*.{jsx,tsx}: When using 'text-right' utility class, always add 'text-balance' to prevent poor text layout
Think about mobile responsiveness when doing any UI tasks or changes
Always use 'rounded' Tailwind class, never 'rounded-xl' or 'rounded-md'
Don't use lucide for icons, ONLY use phosphor icons. Use width='duotone' for most icons, use fill for arrows, and don't add width attribute for plus icons
Decouple state management, data transformations, and API interactions from the React lifecycle in View Components
Simplify data flow to eliminate prop drilling and callback hell in components
Prioritize modularity and testability in all components
ALWAYS use error boundaries properly in React applications
Use 'Icon' suffix at the end of phosphor react icon imports, like CaretIcon not Caret. This is the default import, NOT as a named import
Almost NEVER use useEffect unless it's critical
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-MUST-DO.mdc)
**/*.{js,jsx,ts,tsx}: Split off components, utils, and reusable code to ensure better loading speed and less complexity
Use lower-case-like-this naming convention for variables, functions, and identifiers
NEVER add placeholders, mock data, or anything similar to production code
Use Dayjs for date handling, NEVER use date-fns. Use Tanstack query for hooks, NEVER use SWR
Use json.stringify() when adding debugging code
Never use barrel exports or create index files
Files:
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
🔇 Additional comments (1)
apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx (1)
132-132: LGTM: Improved contrast.The change from
bg-accenttobg-backgroundprovides better visual separation between the header and list sections of the Top Countries panel.
| style={{ minHeight: 350 }} | ||
| > | ||
| <div className="relative min-h-[200px] flex-1 lg:min-h-0"> | ||
| <div className="relative flex-1 max-lg:aspect-video lg:min-h-0 [&>div]:rounded-t-none [&>div]:border-0"> |
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.
🧹 Nitpick | 🔵 Trivial
Consider replacing inline style with Tailwind utility.
The inline style={{ minHeight: 350 }} could be replaced with Tailwind's min-h-[350px] class for consistency with the rest of the styling approach.
♻️ Suggested refactor
- <div
- className="relative flex flex-col lg:flex-row"
- style={{ minHeight: 350 }}
- >
+ <div className="relative flex min-h-[350px] flex-col lg:flex-row">🤖 Prompt for AI Agents
In
@apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
around lines 114 - 116, Replace the inline style on the container element that
currently uses style={{ minHeight: 350 }} with Tailwind utility min-h-[350px];
locate the JSX element in geo-map-section.tsx (the div that wraps the map and
has the inline style) and remove the style prop, then add min-h-[350px] into
that element's className string (merging with existing classes like "relative
flex-1 max-lg:aspect-video lg:min-h-0 [&>div]:rounded-t-none [&>div]:border-0")
so there are no duplicate className props or leftover inline styles.
|
|
||
| <div className="w-full shrink-0 border-t bg-card lg:w-44 lg:border-t-0 lg:border-l"> | ||
| <div className="h-10 border-b bg-card px-2"> | ||
| <div className="absolute right-2 bottom-2 z-1 w-44 shrink-0 rounded border border-t bg-muted"> |
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.
Fix invalid Tailwind z-index class.
The z-1 class is not a valid Tailwind utility. Tailwind provides z-0, z-10, z-20, z-30, z-40, z-50, and z-auto. Unless you have a custom z-1 class defined in your Tailwind config, this will not apply any z-index styling.
Details
🔧 Proposed fix- <div className="absolute right-2 bottom-2 z-1 w-44 shrink-0 rounded border border-t bg-muted">
+ <div className="absolute right-2 bottom-2 z-10 w-44 shrink-0 rounded border border-t bg-muted">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="absolute right-2 bottom-2 z-1 w-44 shrink-0 rounded border border-t bg-muted"> | |
| <div className="absolute right-2 bottom-2 z-10 w-44 shrink-0 rounded border border-t bg-muted"> |
🤖 Prompt for AI Agents
In
@apps/dashboard/app/(main)/websites/[id]/_components/tabs/overview/_components/geo-map-section.tsx
at line 124, The div using className "absolute right-2 bottom-2 z-1 w-44
shrink-0 rounded border border-t bg-muted" uses an invalid Tailwind class `z-1`;
update that class in the GeoMapSection component to a valid Tailwind z-index
utility (e.g., replace `z-1` with `z-10` or `z-0` depending on desired stacking)
or, if you truly need a custom value, add a `z` entry to the Tailwind config
instead and reference that custom class; ensure you update the className string
in the component where `z-1` appears (no other changes needed).
|
Pushed the changes and updated the screenshots in PR description |



Description
Before (Terms checkbox alignment):
After (Terms checkbox alignment):
Before (Visitor Locations card UI on mobile):
After (Visitor Locations card UI on mobile):
Before (Visitor Locations card UI on desktop):
After (Visitor Locations card UI on desktop):
Before (Keyboard Shortcuts UI):
After (Keyboard Shortcuts UI):
Before (Low contrast Keyboard Shortcuts UI):
After (Better contrast Keyboard Shortcuts UI):
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.