new-log-viewer: In the menubar, show file name with a centered ellipsis when the name overflows (Fixes #97). #98
Conversation
…hen the name overflows.
WalkthroughThe changes introduced in this pull request involve modifications to the CSS and TypeScript files of the MenuBar component in the new-log-viewer project. A new CSS class Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (3)
new-log-viewer/src/components/MenuBar/index.css (2)
25-30: Improved filename container styling and layoutThe changes to the
.menu-bar-filename-containerclass (renamed from.menu-bar-filename) effectively address the issue of long filenames overflowing. The addition ofoverflow-x: hidden;anddisplay: flex;provides better control over the layout and prevents horizontal overflow.The padding change to
padding-inline: 0.75rem !important;ensures consistent padding on both sides. However, consider if the!importantdeclaration is absolutely necessary, as it can make future style overrides more difficult.If possible, try to avoid using
!importantby restructuring the CSS specificity instead.
37-40: Effective styling for right part of filenameThe new
.menu-bar-filename-right-splitclass effectively manages the right part of the filename display. The combination ofoverflow-x: hidden;,display: flex, andjustify-content: flex-end;ensures that the right part of the filename is properly aligned and doesn't overflow.However, consider if the
!importantdeclaration ondisplay: flexis absolutely necessary. If possible, try to achieve the same result without using!importantby restructuring the CSS specificity.Evaluate if the
!importantdeclaration can be removed without affecting the desired layout.new-log-viewer/src/components/MenuBar/index.tsx (1)
63-73: Excellent implementation of file name displayThe new implementation effectively addresses the issue of long file names overflowing the menubar. Splitting the file name into two halves and using separate
Typographycomponents allows for displaying both the beginning and end of the file name, which is particularly useful for long names.The
titleattribute on theBoxcomponent ensures that the full file name is accessible on hover, enhancing usability.Suggestion for improvement:
Consider adding a visual indicator (like an ellipsis) between the two halves of the file name when it's truncated. This could be achieved by adding a thirdTypographycomponent with a fixed width containing an ellipsis, placed between the two existingTypographycomponents.Would you like me to provide an example implementation of this suggestion?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- new-log-viewer/src/components/MenuBar/PageNumInput.css (1 hunks)
- new-log-viewer/src/components/MenuBar/index.css (1 hunks)
- new-log-viewer/src/components/MenuBar/index.tsx (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- new-log-viewer/src/components/MenuBar/PageNumInput.css
🧰 Additional context used
🔇 Additional comments (5)
new-log-viewer/src/components/MenuBar/index.css (3)
17-17: Improved flexibility for logo container sizingThe change from
width: 48px;tomin-width: 48px;allows for more flexible sizing of the logo container while maintaining a minimum width. This adjustment aligns well with the PR objective of improving the layout for long filenames.
32-35: Effective styling for left part of filenameThe new
.menu-bar-filename-left-splitclass effectively manages the left part of the filename display. The combination ofoverflow-x: hidden;andtext-overflow: ellipsis;ensures that long filenames are truncated with an ellipsis, preventing overflow and maintaining a clean layout. This implementation aligns perfectly with the PR objective of showing the filename with a centered ellipsis when it overflows.
17-40: Effective implementation of centered ellipsis for long filenamesThe changes in this file successfully implement the solution for displaying long filenames with a centered ellipsis in the menubar. The combination of flexbox layout, overflow management, and the split of the filename into left and right parts effectively addresses the issue described in #97.
These changes align well with the PR objectives:
- They prevent filename overflow in the menubar.
- They ensure that both the beginning and end of long filenames remain visible.
- They allow for dynamic adjustment based on available space.
The implementation improves the user experience and maintains a clean, responsive layout. Great job on addressing this issue!
new-log-viewer/src/components/MenuBar/index.tsx (2)
4-4: LGTM: Import statement added for Box componentThe addition of the
Boximport from@mui/joyis appropriate for the changes made in the component's rendering logic.
Line range hint
1-89: Overall impact: Effective solution for file name overflowThe changes implemented in this file successfully address the issue of long file names overflowing the menubar, as described in the linked issue #97. The new approach of splitting the file name display allows users to see both the beginning and end of long file names, improving usability.
The modifications are well-contained within the
MenuBarcomponent and don't appear to introduce any breaking changes or unintended side effects.To ensure the changes work as expected across different scenarios:
Please run the following script to check for any potential regressions or unexpected behaviour:
This script will help identify any potential conflicts or areas that might need adjustment following these changes.
✅ Verification successful
Verification Successful: No Issues Found
The implemented changes to the
MenuBarcomponent do not introduce any regressions or conflicts. The component is only imported inLayout.tsx, and there are no hardcoded width values or other implementations affecting file name display.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of MenuBar component and potential impacts # Test 1: Check if MenuBar is imported and used correctly in other components echo "Checking MenuBar usage:" rg --type typescript "import.*MenuBar" -g '!**/MenuBar/**' # Test 2: Verify if there are any hardcoded width values for file name display that might conflict with the new implementation echo "Checking for hardcoded file name widths:" rg --type typescript --type css "width.*file.*name" -g '!**/MenuBar/**' # Test 3: Look for any other components that might be handling file name display echo "Checking for other file name display implementations:" rg --type typescript "fileName.*display|display.*fileName" -g '!**/MenuBar/**'Length of output: 585
Script:
#!/bin/bash # Description: Verify the usage of MenuBar component and potential impacts # Test 1: Check if MenuBar is imported and used correctly in other components echo "Checking MenuBar usage:" rg "import.*MenuBar" --glob '!**/MenuBar/**' --glob '*.ts' --glob '*.tsx' # Test 2: Verify if there are any hardcoded width values for file name display that might conflict with the new implementation echo "Checking for hardcoded file name widths:" rg "width.*file.*name" --glob '!**/MenuBar/**' --glob '*.ts' --glob '*.tsx' rg "width.*file.*name" --glob '!**/MenuBar/**' --glob '*.css' # Test 3: Look for any other components that might be handling file name display echo "Checking for other file name display implementations:" rg "fileName.*display|display.*fileName" --glob '!**/MenuBar/**' --glob '*.ts' --glob '*.tsx'Length of output: 652
| @@ -14,15 +14,28 @@ | |||
| .menu-bar-logo-container { | |||
| display: flex; | |||
| justify-content: center; | |||
| width: 48px; | |||
| min-width: 48px; | |||
There was a problem hiding this comment.
This avoids the YScope logo container from being squeezed when the other items in the menu bar try to take space.
junhaoliao
changed the title
junhaoliao
changed the title
junhaoliao
changed the title
References
Fixes #97
new-log-viewer series: #45 #46 #48 #51 #52 #53 #54 #55 #56 #59 #60 #61 #62 #63 #66 #67 #68 #69 #70 #71 #72 #73 #74 #76 #77 #78 #79 #80 #81 #82 #83 #84 #89 #91 #93 #94 #96
Description
Validation performed
Summary by CodeRabbit
New Features
Improvements
Bug Fixes