[Fix] Dark colors | 'groupBy' report limits dropdown #3469
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/app/[locale]/reports/weekly-limit/components/group-by-select.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe pull request focuses on enhancing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/app/[locale]/reports/weekly-limit/components/group-by-select.tsx (3)
90-92: Update comment to reflect single-selection behaviorThe comment suggests multi-selection behavior ("clearing all selections"), but according to the learnings, only one option can be selected at a time. Please update the comment to accurately reflect the component's behavior.
-// Prevents users from clearing all selections, ensuring at least one option is always selected. +// Prevents deselection of the only selected option, as one option must always be selected.
94-99: Improve className implementation readabilityThe className implementation could be more maintainable by extracting the conditional classes into a separate constant or using a utility like
clsxortailwind-merge.-className={({ active, selected }) => - `relative cursor-default select-none py-2 pl-10 pr-4 ${ - active - ? 'bg-primary/10 text-primary dark:text-white dark:bg-primary/10' - : 'text-gray-900 dark:text-white' - } ${selected && 'dark:bg-primary/10'}` -} +className={({ active, selected }) => { + const baseClasses = 'relative cursor-default select-none py-2 pl-10 pr-4'; + const activeClasses = active + ? 'bg-primary/10 text-primary dark:text-white dark:bg-primary/10' + : 'text-gray-900 dark:text-white'; + const selectedClasses = selected ? 'dark:bg-primary/10' : ''; + + return `${baseClasses} ${activeClasses} ${selectedClasses}`; +}}
Line range hint
105-105: Consider consistent text casingThe option value is lowercase but displayed capitalized. Consider either:
- Storing the values in the desired display case, or
- Adding a text transform class to handle the display formatting
-<span className={`block truncate ${selected ? 'font-medium' : 'font-normal'}`}> +<span className={`block truncate capitalize ${selected ? 'font-medium' : 'font-normal'}`}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/[locale]/reports/weekly-limit/components/group-by-select.tsx(2 hunks)
🧰 Additional context used
📓 Learnings (1)
apps/web/app/[locale]/reports/weekly-limit/components/group-by-select.tsx (1)
Learnt from: CREDO23
PR: ever-co/ever-teams#3392
File: apps/web/app/[locale]/reports/weekly-limit/components/group-by-select.tsx:42-51
Timestamp: 2024-12-05T09:04:08.049Z
Learning: In the `GroupBySelect` component (`apps/web/app/[locale]/reports/weekly-limit/components/group-by-select.tsx`), only one option can be selected at a time; two options cannot be selected simultaneously.
🔇 Additional comments (2)
apps/web/app/[locale]/reports/weekly-limit/components/group-by-select.tsx (2)
62-62: LGTM: Dark mode styling for button is properly implemented
The dark mode classes are correctly applied to the Listbox.Button, providing appropriate contrast for both border and background colors while maintaining focus states.
86-86: LGTM: Dropdown options container properly styled for dark mode
The dark mode background is consistently applied to the options container, maintaining visual hierarchy with appropriate z-index and shadow styling.
evereq
deleted the
3466-bug-weekly-limit--group-by-dropdown-dark-colors
branch
Description
Fixes #3466
Type of Change
Checklist
Previous screenshots
Current screenshots
Summary by CodeRabbit
New Features
Bug Fixes