BUG : Fix misalignment of zoom slider circle in toolbar and made it acc to req ui #604
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
""" WalkthroughThe CSS for the zoom slider in The Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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
npm error Exit handler never called! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
@ThatDeparted2061 pls review the pr , thanks |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/Navbar/QuickButton/QuickButton.vue (3)
215-232: Add Keyboard Focus Indicators
The decrement/increment buttons lack a visible focus style for keyboard users. Introduce a:focus-visiblerule (e.g., outline or box-shadow) to ensure accessibility.
252-260: Consider Hover State for Thumb
The 12×12 px thumb with box-shadow is well-styled. You may want to add a::-moz-range-thumb:hoverrule for additional hover feedback.
281-292: Use Transform for Vertical Centering
Hardcodedmargin-top: -5pxmay break if track height changes. Preferposition: relative; top: 50%; transform: translateY(-50%);for more resilient centering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Navbar/QuickButton/QuickButton.vue(2 hunks)
🔇 Additional comments (5)
src/components/Navbar/QuickButton/QuickButton.vue (5)
206-213: CSS Adjustment Approved
The revised.zoom-sliderflex layout, reduced font size, and alignment properties correctly center the slider controls and match the design spec.
234-237: Hover State Enhancement Confirmed
The opacity transition on hover provides clear feedback without affecting layout.
245-250: Firefox Track Styling OK
The translucent 2 px track in Firefox matches the WebKit version and aligns with the UI requirements.
262-264: Verify Focus Visibility
The focus style removes the outline but relies on a subtle box-shadow. Confirm that this meets your accessibility contrast requirements.
274-279: WebKit Track Styling Matches
The 2 px white track in WebKit browsers is consistent with Firefox and aligns with the design.
|
@blazethunderstorm check the changes for small screen and address the coderabbit suggestions. Then I'll approve it |
|
@ThatDeparted2061 i have made req changes |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/Navbar/QuickButton/QuickButtonMobile.vue (1)
69-69: Nitpick: Inline styles should be moved to CSS classes.Consider moving the inline style to a CSS class for better maintainability.
- <i style="color: #ddd" class="fas fa-expand-arrows-alt"></i> + <i class="fas fa-expand-arrows-alt view-icon"></i>Add corresponding CSS:
.view-icon { color: #ddd; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Navbar/QuickButton/QuickButton.vue(2 hunks)src/components/Navbar/QuickButton/QuickButtonMobile.vue(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Navbar/QuickButton/QuickButton.vue
🔇 Additional comments (9)
src/components/Navbar/QuickButton/QuickButtonMobile.vue (9)
2-5: LGTM: Clean template restructuring with drag support.The template has been well-restructured with a single container and proper drag functionality. The drag panel implementation looks correct.
72-84: LGTM: Zoom slider structure maintained.The zoom slider HTML structure is preserved correctly with proper input attributes and button controls.
92-98: LGTM: Proper drag event handling.The dragover function correctly resets the styling when drag events occur. The implementation is clean and follows good practices.
245-254: Excellent: Proper Firefox zoom slider thumb alignment fix.The Mozilla range thumb styling correctly addresses the misalignment issue mentioned in the PR. The 12px width/height with border-radius: 50% creates a perfectly circular thumb, and the styling ensures proper visual consistency.
273-284: Excellent: WebKit zoom slider thumb alignment fix addresses the core issue.The key fix is on line 281 with
margin-top: -5pxwhich properly centers the 12px circular thumb on the 2px track. This directly addresses the misalignment issue mentioned in the PR objectives.The calculation is correct: (12px thumb height - 2px track height) / 2 = 5px offset needed.
238-243: LGTM: Consistent track styling for Firefox.The Mozilla range track styling with 2px height and proper background color ensures visual consistency with the WebKit implementation.
266-271: LGTM: WebKit track styling matches Firefox implementation.The webkit slider track with 2px height matches the Mozilla implementation, ensuring cross-browser consistency.
290-347: LGTM: Responsive design for smaller screens.The media query properly scales button sizes for screens under 991px width, ensuring good mobile experience.
101-101: ```shell
#!/bin/bashExtract the <style> block from QuickButtonMobile.vue to list its defined CSS classes
sed -n '1,200p' src/components/Navbar/QuickButton/QuickButtonMobile.vue | sed -n '/<style/,/</style>/p'
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| .quick-btn { | ||
| display: flex; | ||
| align-items: center; | ||
| z-index: 99; | ||
| justify-content: space-between; | ||
| padding: 0 1.5rem; | ||
| position: absolute; | ||
| width: 400px; | ||
| height: 33px; | ||
| top: 90px; | ||
| right: 280px; | ||
| border-radius: 7px; | ||
| z-index: 100; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review absolute positioning values for responsiveness.
The fixed positioning values (width: 400px, top: 90px, right: 280px) might not work well across all mobile screen sizes. Consider using responsive units or media queries.
.quick-btn {
display: flex;
position: absolute;
- width: 400px;
+ width: min(400px, calc(100vw - 40px));
height: 33px;
- top: 90px;
- right: 280px;
+ top: 5rem;
+ right: min(280px, 5vw);
border-radius: 7px;
z-index: 100;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .quick-btn { | |
| display: flex; | |
| align-items: center; | |
| z-index: 99; | |
| justify-content: space-between; | |
| padding: 0 1.5rem; | |
| position: absolute; | |
| width: 400px; | |
| height: 33px; | |
| top: 90px; | |
| right: 280px; | |
| border-radius: 7px; | |
| z-index: 100; | |
| } | |
| .quick-btn { | |
| display: flex; | |
| position: absolute; | |
| width: min(400px, calc(100vw - 40px)); | |
| height: 33px; | |
| top: 5rem; | |
| right: min(280px, 5vw); | |
| border-radius: 7px; | |
| z-index: 100; | |
| } |
🤖 Prompt for AI Agents
In src/components/Navbar/QuickButton/QuickButtonMobile.vue around lines 115 to
124, the CSS uses fixed pixel values for width, top, and right properties which
can cause layout issues on different mobile screen sizes. Replace these fixed
pixel values with responsive units like percentages, vw, or use media queries to
adjust the positioning and size based on screen width to ensure the button
displays correctly on all devices.
|
@ThatDeparted2061 pls review |
|
Where did you see this slider issue, i dont see it in tauri and web simulator ? |
|
@ThatDeparted2061 in web simulator |
|
Its there |
Fixes #603
Describe the changes you have made in this PR -
Fixed the alignment issue of the zoom slider thumb within the quick action toolbar. The circle (thumb) was previously slightly misaligned vertically on both desktop and mobile views. Updated the CSS styles for ::-webkit-slider-thumb and ::-moz-range-thumb to ensure proper centering across browsers and screen sizes. This improves the visual consistency and polish of the zoom control component.
Screenshots of the changes (If any) -
Before ---
After ---
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
cc @ThatDeparted2061 @Arnabdaz @niladrix719