fix(AppFrame): Use dynamic view height in app frame #1440
Conversation
WalkthroughThe changes involve updating the CSS in the Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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
🧹 Outside diff range and nitpick comments (1)
packages/react-components/src/components/AppFrame/AppFrame.module.scss (1)
8-8: Consider adding a fallback for older browsersWhile
dvhis well-supported in modern browsers, consider adding a fallback:.#{$base-class} { + height: 100vh; /* Fallback for older browsers */ height: 100dvh;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/react-components/src/components/AppFrame/AppFrame.module.scss(1 hunks)
🔇 Additional comments (1)
packages/react-components/src/components/AppFrame/AppFrame.module.scss (1)
8-8: LGTM! Good choice using dvh
The change from vh to dvh is an excellent solution for the iPad scrolling issues. Dynamic viewport height (dvh) accounts for dynamic UI elements like address bars, providing a more stable layout on mobile devices.
Let's check browser support for dvh:
✅ Verification successful
dvh usage is safe and appropriate
The change to dvh is the only instance in the codebase, replacing the previous vh unit. Since this is a modern design system (using React 17+, Vite, and modern tooling), the browser support for dvh (92.71% global coverage) is sufficient for the target audience.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for dvh unit in browserslist config
fd -t f "package.json|.browserslistrc" -x cat {} | grep -i "browserslist"
# Check if there are any CSS fallbacks for dvh
rg -i "(?:height:\s*100vh|@supports.*dvh)"
Length of output: 120
Script:
#!/bin/bash
# Let's check package.json files for browserslist config
fd "package.json" -x cat {}
# Check for any existing vh units in SCSS files
rg -i "height:\s*\d+vh" -g "*.scss"
# Check for any dvh units in SCSS files
rg -i "height:\s*\d+dvh" -g "*.scss"
Length of output: 7066
rafalluksza
changed the title
Resolves: LC-3261
Description
Use dynamic 100dvh to prevent scroll on mobile devices.
Storybook
https://feature-lc-3261--613a8e945a5665003a05113b.chromatic.com
Checklist
Obligatory:
Summary by CodeRabbit