Fix viewport min/max button in Safari #8337
Conversation
📝 WalkthroughWalkthroughThe pull request introduces various improvements to the WEBKNOSSOS frontend, focusing on UI enhancements, styling updates, and bug fixes. The changes primarily target the flex layout styling, addressing viewport interactions, tab styling, and splitter behaviors. Specifically, new CSS classes were added to improve the visual and interactive aspects of the application's layout, with particular attention to Safari-specific viewport maximization issues. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
🧹 Nitpick comments (2)
frontend/stylesheets/flex_layout_overwrites.less (1)
Line range hint
31-61: LGTM! Consider documenting the z-index value.The changes effectively address the Safari viewport issue by:
- Using z-index to control stacking order
- Implementing transparent pseudo-elements to increase click targets
- Properly positioning pseudo-elements for both vertical and horizontal splitters
Consider adding a comment explaining why z-index: 21 was chosen to help future maintainers understand its relationship with other z-index values in the application.
CHANGELOG.unreleased.md (1)
55-55: Consider adding more detail to the changelog entry.While the entry correctly describes the fix, it could be more helpful to future readers by mentioning the root cause (FlexLayout splitter overlay issue) and the affected browsers (Safari 18.1.1+).
-Fixed an issue where you could not maximize or reposition the 3D/XZ viewport in Safari. [#8337](https://github.com/scalableminds/webknossos/pull/8337) +Fixed an issue where you could not maximize or reposition the 3D/XZ viewport in Safari (18.1.1+) due to FlexLayout splitter overlaying the tab header. [#8337](https://github.com/scalableminds/webknossos/pull/8337)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.unreleased.md(1 hunks)frontend/stylesheets/flex_layout_overwrites.less(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: circleci_build
🔇 Additional comments (3)
frontend/stylesheets/flex_layout_overwrites.less (3)
65-69: LGTM! Z-index hierarchy properly established.The z-index: 100 ensures the tab bar remains above the splitter (z-index: 21), fixing both the viewport button clickability and tab repositioning in Safari.
Line range hint
96-126: LGTM! Consistent use of design system variables.The tab button styling changes maintain visual consistency while improving user feedback through hover states and selected states.
150-154: LGTM! Splitter interaction behavior properly maintained.The selector changes correctly maintain the intended behavior where the first splitter is non-interactive while subsequent splitters remain interactive.
2 participants
This PR fixes an issue with incorrect mouse input handlers in Safari. Some FlexLayout functionalities did not work correctly in Safari:
The FlexLayout horizontal
splitter"overlaid" the tab header preventing all mouse interactions (or more precisely all mouse inputs started the window/tab resizing - caused by the splitter's event handler).This regression was likely caused by an update to FlexLayout in the past when they renamed some of their CSS classes which we did not update in our overwrites correctly.
Tested with Safari 18.1.1, Chrome 131.0.6778.86 and Firefox 134.0.1.
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)