(apps/web)[Fix]: Assignees and Project Dropdown on TaskInput #3374
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested reviewers
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/components/pages/task/details-section/blocks/task-secondary-info.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. 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: 2
🧹 Outside diff range and nitpick comments (3)
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (2)
Line range hint
70-200: Remove duplicate sections for Epic, Task Status, and Task LabelsThere are multiple instances of duplicated code sections which should be removed:
- Epic section is duplicated
- Task Status section is duplicated
- Task Labels section is duplicated
This duplication increases maintenance burden and the likelihood of inconsistencies.
Remove the duplicate sections and keep only one instance of each. Here's a partial diff showing the removal of duplicates:
{/* Epic */} {task && task.issueType === 'Story' && ( <TaskRow labelTitle={t('pages.taskDetails.EPIC')}> <TaskEpicDropdown onValueChange={(d) => { onTaskSelect({ id: d } as ITeamTask); }} className="lg:min-w-[170px] text-black" forDetails={true} sidebarUI={true} taskStatusClassName="text-[0.625rem] w-[7.6875rem] h-[2.35rem] max-w-[7.6875rem] rounded 3xl:text-xs" defaultValue={task.parentId || ''} /> </TaskRow> )} -{/* Epic */} -{task && task.issueType === 'Story' && ( - <TaskRow labelTitle={t('pages.taskDetails.EPIC')}> - <TaskEpicDropdown - onValueChange={(d) => { - onTaskSelect({ - id: d - } as ITeamTask); - }} - className="lg:min-w-[170px] text-black" - forDetails={true} - sidebarUI={true} - taskStatusClassName="text-[0.625rem] w-[7.6875rem] h-[2.35rem] max-w-[7.6875rem] rounded 3xl:text-xs" - defaultValue={task.parentId || ''} - /> - </TaskRow> -)}
Line range hint
466-475: Enhance error handling in project updatesWhile the error handling is present, it could be improved by:
- Adding user feedback for failed operations
- Reverting the selected state on error
Consider updating the error handling:
const handleUpdateProject = useCallback( async (project: IProject) => { try { if (task) { await updateTask({ ...task, projectId: project.id }); } } catch (error) { + // Revert selection on error + setSelected(organizationProjects.find(p => p.id === task?.projectId)); + // Show error notification to user + console.error('Failed to update project:', error); - console.error(error); } }, [task, updateTask] );apps/web/lib/features/task/task-input.tsx (1)
754-759: Consider extracting common dark mode styles into a shared class.The dark mode styles are repeated across multiple elements. Consider extracting them into a shared Tailwind class to improve maintainability.
+ // Create a new class in your Tailwind config + // 'dark-mode-container': 'dark:bg-[#1B1D22] dark:border dark:border-[#FFFFFF33]' - <div className="w-52 rounded-xl bg-[#F2F2F2] dark:bg-[#1B1D22] dark:border dark:border-[#FFFFFF33] py-2 px-3"> + <div className="w-52 rounded-xl bg-[#F2F2F2] dark-mode-container py-2 px-3">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx(2 hunks)apps/web/lib/features/task/task-input.tsx(3 hunks)
🧰 Additional context used
📓 Learnings (1)
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (1)
Learnt from: CREDO23
PR: ever-co/ever-teams#3360
File: apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx:346-354
Timestamp: 2024-11-25T18:49:15.126Z
Learning: In `apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx`, within the `ProjectDropDown` component, both parent and child state should be synchronized for better UI updates, even in controlled mode. The `onChange` function carries the parent state, and `setSelected` maintains the selected state in the child component.
🔇 Additional comments (2)
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (1)
421-424: LGTM: Improved styling for ProjectDropDown
The styling changes enhance dark mode support and visual consistency:
- Added proper dark mode background and border colors
- Improved button layout with flex positioning
- Enhanced ChevronDownIcon transitions
Also applies to: 442-442
apps/web/lib/features/task/task-input.tsx (1)
759-759: 🛠️ Refactor suggestion
Consider adding keyboard navigation support.
The dropdown implementation could benefit from keyboard navigation support for better accessibility. Consider handling arrow keys and Enter/Escape keys for navigation.
Also applies to: 770-770
|
@Cedric921 I don't get it, why all those inputs show NOTHING? How user suppose to know which one for what!? It was before written some text there like "Size" or "Labels" or "Priority" or something (unless some values are selected)... I don't think you changed that in your PR, but how you could NOT fix that!??? from your own video: |
Description
Please include a summary of the changes and the related issue.
Type of Change
Checklist
Previous screenshots
Screen.Recording.2024-11-26.at.09.02.16.mov
Current screenshots
Screen.Recording.2024-11-26.at.11.17.53.mov
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores