[WEB-2552] fix: issue list overflow and event propagation#5706
[WEB-2552] fix: issue list overflow and event propagation#5706
Conversation
WalkthroughThe changes across multiple components primarily focus on enhancing the rendering and interaction of issue names within modals and lists. Modifications include the addition of truncation classes for better text overflow handling, the implementation of Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
🧹 Outside diff range and nitpick comments (5)
web/core/components/issues/relations/issue-list-item.tsx (2)
98-98: LGTM! Consider adding text truncation.The removal of
line-clamp-1addresses the overflow issue mentioned in the PR objectives. However, to ensure long issue names don't overflow, consider adding atruncateclass to this element or its child elements.- className="w-full cursor-pointer" + className="w-full cursor-pointer truncate"
126-132: LGTM! Effective solution for event propagation issues.The addition of the
onClickevent handler that prevents default actions and stops event propagation effectively addresses the issues mentioned in the PR objectives. This will prevent unwanted redirections when interacting with sub-issues or relations.Consider extracting the event handler to a separate function for improved readability:
+ const handleRelationIssuePropertyClick = (e: React.MouseEvent) => { + e.preventDefault(); + e.stopPropagation(); + }; + // ... (inside the component's return statement) <div className="flex-shrink-0 text-sm" - onClick={(e) => { - e.preventDefault(); - e.stopPropagation(); - }} + onClick={handleRelationIssuePropertyClick} >web/core/components/issues/sub-issues/issue-list-item.tsx (1)
153-159: Approve IssueProperty wrapper and suggest minor improvementThe addition of a wrapper div with event propagation control around the IssueProperty component is a good improvement. This change addresses the PR objective of controlling event propagation for quick actions in the issue layout, enhancing the user experience.
However, consider extracting the inline click handler to a separate function for better readability and maintainability.
Consider refactoring the click handler as follows:
const handleIssuePropertyClick = (e: React.MouseEvent) => { e.preventDefault(); e.stopPropagation(); }; // Then in the JSX: <div className="flex-shrink-0 text-sm" onClick={handleIssuePropertyClick} > <IssueProperty // ... props ... /> </div>This refactoring will improve code readability and make it easier to add any additional logic to the handler in the future if needed.
web/core/components/issues/issue-layouts/list/block.tsx (1)
274-277: LGTM! Consider adding a comment for clarity.The addition of this
onClickhandler effectively addresses the issue of unwanted redirection when interacting with sub-issues, as outlined in the PR objectives. By preventing the default action and stopping event propagation, it ensures that clicks on this element don't trigger unintended navigation.Consider adding a brief comment explaining the purpose of this event handler for future maintainability:
onClick={(e) => { e.preventDefault(); e.stopPropagation(); + // Prevent navigation when interacting with quick actions }}web/core/components/core/modals/existing-issues-list-modal.tsx (1)
Line range hint
1-324: Consider enhancing accessibility with aria-labelsThe component implementation looks good overall and aligns well with the PR objectives. To further improve accessibility, consider adding aria-labels to key interactive elements, such as the search input and the workspace level toggle switch. This will provide better context for screen reader users.
Example:
<Combobox.Input aria-label="Search for issues" // ... other props /> <ToggleSwitch aria-label="Toggle workspace level search" // ... other props />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- web/core/components/core/modals/existing-issues-list-modal.tsx (2 hunks)
- web/core/components/issues/issue-layouts/list/block.tsx (1 hunks)
- web/core/components/issues/relations/issue-list-item.tsx (2 hunks)
- web/core/components/issues/sub-issues/issue-list-item.tsx (5 hunks)
🔇 Additional comments (11)
web/core/components/issues/relations/issue-list-item.tsx (3)
103-103: LGTM! Effective solution for text overflow.The addition of the
truncateclass effectively addresses the issue of overflowing text mentioned in the PR objectives. Retaining thecursor-pointerclass maintains the interactive nature of the element.
123-123: LGTM! Improved handling of long issue names.The addition of
w-fullandtruncateclasses effectively addresses the issue of overflowing text for long issue names, as mentioned in the PR objectives. This change ensures better readability and consistent layout.
Line range hint
1-185: Overall, great implementation addressing the PR objectives.The changes effectively solve the issues of text overflow and event propagation mentioned in the PR objectives. The code quality is good, with clear and purposeful modifications. The suggestions provided are minor and aimed at further improving readability and consistency.
Remember to test the changes thoroughly, especially focusing on:
- The display of long issue names in various sections.
- The behavior of sub-issues and relations when interacting with them.
- The functionality of quick actions in the issue list layout.
Great job on addressing these UI/UX improvements!
web/core/components/issues/sub-issues/issue-list-item.tsx (5)
89-90: Approve change and verify overflow behaviorThe removal of the
line-clamp-1class from the ControlLink component aligns with the PR objective of addressing overflow issues. This change should help prevent the overflow of issue names in the sub-issue and relation sections.Please verify that this change effectively resolves the overflow issue without introducing any unintended side effects in the UI.
107-109: Approve event propagation controlThe addition of
e.preventDefault()ande.stopPropagation()in the click event handler effectively addresses the PR objective of preventing unwanted redirection when updating or toggling sub-issues. This change enhances the user experience by ensuring that clicks on sub-issues don't trigger unintended actions in parent components.
130-130: Approve truncate class additionThe addition of the
truncateclass to the issue name container is a good improvement. This change enhances text overflow management and directly addresses the PR objective of preventing the overflow of issue names in the sub-issue and relation sections.
149-149: Approve tooltip span improvementsThe addition of the
truncateclass and the specific text color class (text-custom-text-100) to the tooltip span for the issue name is a good improvement. These changes enhance visual consistency, improve readability, and further address the overflow issue mentioned in the PR objectives.
Line range hint
1-255: Summary of changes and overall approvalThe changes made to the
IssueListItemcomponent effectively address the PR objectives:
- Overflow issues have been resolved by removing
line-clamp-1and addingtruncateclasses.- Unwanted redirection has been prevented by adding event propagation control.
- Quick actions in the issue layout now have proper event handling.
These modifications improve the overall functionality and user experience of the issue list item component. The code changes are well-implemented and align with the PR's goals.
web/core/components/issues/issue-layouts/list/block.tsx (1)
Line range hint
1-290: Overall implementation looks good. Verify with QA team.The changes made to the
IssueBlockcomponent effectively address the issue of unwanted redirection when updating or toggling sub-issues, as outlined in the PR objectives. The implementation is minimal and focused, without introducing any apparent side effects or breaking changes.Please ensure that the QA team thoroughly tests the following scenarios:
- Updating sub-issues in the Issue Sub-issue section of the Issue Peek Overview.
- Toggling sub-issues in the Issue Sub-issue section.
- Interacting with quick actions in the issue list layout, particularly the edit and delete modals.
To assist with verification, you can use the following script to locate all usage of the
IssueBlockcomponent:This will help ensure that the changes have the intended effect across all instances where the
IssueBlockcomponent is used.web/core/components/core/modals/existing-issues-list-modal.tsx (2)
Line range hint
252-269: LGTM: Improved text truncation for issue namesThe addition of the
truncateclass to both the containerdivand the issue namespaneffectively addresses the overflow issue mentioned in the PR objectives. This change ensures that long issue names will be truncated appropriately, improving the overall layout and readability of the issue list.
274-274: LGTM: Improved layout for issue link iconThe addition of
flex-shrink-0to theatag prevents the link icon from being compressed when the issue name is long. This change ensures that the icon remains visible and accessible, contributing to the overall improvement of the layout and user experience.
3 participants
Problem:
Solution:
Areas to Test (For QA Team)
Reference:
[WEB-2552]
Media: