fix(flyout): support dynamic heights on mobile devices - #3528
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesThe pull request raises browser minimum versions in documentation and build configuration, changes flyout and modal sizing calculations to use Browser and viewport updates
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 63: Correct the browser name typo in the support documentation list by
changing “Saumsung Internet” to “Samsung Internet,” while preserving the
existing link, icon, and version requirement.
- Around line 58-65: Remove the empty link wrappers from the browser
compatibility entries in the README, changing each []() pattern to standalone
image syntax while preserving the existing alt text, logo URLs, and version
descriptions.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 92f22272-826d-4f02-9734-81ef65bf5d5b
📒 Files selected for processing (5)
README.mdsrc/definitions/modules/flyout.jssrc/themes/default/modules/flyout.variablessrc/themes/default/modules/modal.variablestasks/config/tasks.js
💤 Files with no reviewable changes (1)
- src/definitions/modules/flyout.js
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
Description
Modern followup of #2644
This is now completely solvable via pure CSS, by using the "dvh" unit supporting dynamic view heights on mobile devices (which removes the address bar on scrolling which in turn changes the height)
The now unnecessary JS code, which would otherwise calculate wrong values for the min height, can be completely removed.
Same applies for fullscreen modal, so this PR adjusts this as well.
This a breaking change as it needs a modern browser (12/2022+, iOS15.4+) supporting the dvh unit
https://caniuse.com/?search=dynamic+viewport+units
Testcase
Try the fomantic docs site for flyout and run the first example on a mobile device. Then scroll up/down and the actions area gets too high.
Summary by CodeRabbit
Bug Fixes
Documentation
Compatibility