-
Notifications
You must be signed in to change notification settings - Fork 404
[fix] Correct WhatsNew popup arrow alignment with help center icon #5137
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
Conversation
🎭 Playwright Test Results✅ All tests passed across all browsers! ⏰ Completed at: 08/27/2025, 07:19:06 AM UTC 📊 Test Reports by Browser
🎉 Your tests are passing across all browsers! |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 08/20/2025, 11:18:34 PM UTC 📊 Build Summary
🔗 Links🎉 Your Storybook is ready for review! |
| var(--sidebar-width, 4rem) + 0.25rem | ||
| ); /* Position toward center of help center icon */ | ||
| var(--sidebar-width, 4rem) * 2 + 0.25rem | ||
| ); /* Position toward center of help center icon (accounting for 2 icons below) */ |
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.
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.
Thank you for the help, let me fix tomorrow
|
@viva-jinyi Can you finish this? |
81e1f2e to
5acbcb2
Compare
@christian-byrne Since the WhatsNew popup was teleported to #graph-canvas-container, it couldn’t access the sidebar CSS variable (--sidebar-width). Moved the --sidebar-width CSS variable to :root so it can be accessed globally.
|
5ca265a to
35a86ac
Compare
|
Do we actually need to pollute global namespace to do that? Can't it be a prop or injected? |
The arrow positioning was not accounting for additional sidebar icons (terminal and shortcuts) that were added below the help center icon, causing misalignment. Updated the calculation to properly position the arrow relative to the help center icon's current location. Fixes #5126
…ment - Fixed small sidebar rule to use consistent calculation with normal sidebar - Updated positioning to use half icon height for better center alignment - Both normal and small sidebar now use dynamic CSS variable calculations Addresses feedback from review by viva-jinyi on CSS specificity and positioning accuracy.
- Move --sidebar-width CSS variable to :root to make it accessible globally - This allows teleported components like WhatsNewPopup to reference sidebar dimensions - Adjust arrow positioning calculations for better alignment with help center icon - Add explanatory comments about why these variables need to be global 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
f544e0a to
f14afb6
Compare
|
@christian-byrne Since the component is teleported elsewhere, we can’t use props or injections. In the current situation, handling it with a CSS variable seems like the fastest and most manageable approach. And since the sidebar width variable is something that can be used at the layout level anyway, I think it’s fine to make it global. WDYT? |



Fixes the WhatsNew popup arrow positioning that was misaligned after new sidebar icons were added below the help center icon. The arrow now correctly points to the help center icon by accounting for the additional icons in its positioning calculation.
Fixes #5126
┆Issue is synchronized with this Notion page by Unito