-
Notifications
You must be signed in to change notification settings - Fork 12
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
new-log-viewer: Fix filename wrapping and overflow in menu bar for filenames with hyphens on narrow windows (fixes #99). #100
Conversation
…enames with hyphens on narrow windows.
WalkthroughThe changes involve modifications to the CSS of the MenuBar component, specifically adding padding and white-space properties to the Changes
Assessment against linked issues
Possibly related PRs
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 (
|
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: 0
🧹 Outside diff range and nitpick comments (1)
new-log-viewer/src/components/MenuBar/index.css (1)
29-32
: The changes effectively address the filename wrapping issue.The addition of
white-space: nowrap;
prevents the filename from wrapping into multiple lines, which solves the overflow problem in narrow windows as described in issue #99. The extra padding also improves the layout.However, consider the following suggestion:
The use of
!important
forpadding-inline
might be overly forceful. If possible, try to achieve the same result without!important
by increasing the specificity of the selector or reorganizing the CSS. This would improve maintainability in the long run.- padding-inline: 0.75rem !important; + padding-inline: 0.75rem;If removing
!important
causes issues, please explain why it's necessary so we can document this decision.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- new-log-viewer/src/components/MenuBar/index.css (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
new-log-viewer/src/components/MenuBar/index.css (1)
29-32
: Verify the fix across different scenariosThe changes look good and should resolve the issue described in #99. To ensure a comprehensive fix:
- Please test this solution in various browsers (Chrome, Firefox, Safari, Edge) to confirm cross-browser compatibility.
- Verify that the fix works for extremely long filenames and very narrow window widths (e.g., mobile devices in portrait mode).
- Ensure that the solution doesn't introduce any new issues, such as horizontal scrolling or content being cut off.
If you've already performed these checks, please share your findings. If not, I'd be happy to assist in creating a testing plan or checklist for thorough verification.
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.
LGTM
References
Fixes #99
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 #98
Description
white-space: nowrap;
to.menu-bar-filename-container
.Validation performed
Summary by CodeRabbit