-
Notifications
You must be signed in to change notification settings - Fork 111
feat(plugin-basic-ui): Parameterize appscreen dim height, appscreen default transition offset #653
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
🦋 Changeset detectedLatest commit: 6dd4ccf The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying stackflow-demo with
|
| Latest commit: |
6dd4ccf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8af63525.stackflow-demo.pages.dev |
| Branch Preview URL: | https://appscree-font-scaling.stackflow-demo.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | 6dd4ccf | Commit Preview URL | Nov 27 2025, 07:51 AM |
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughParameterizes previously hardcoded UI dimensions and transition offsets in plugin-basic-ui by introducing global variables Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
extensions/plugin-*/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (4)
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. Comment |
commit: |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
extensions/plugin-basic-ui/src/basicUIPlugin.css.ts (1)
15-15: Consider using consistent casing:defaultTransitionOffset.The variable name uses "OffSet" with capital 'S', but the conventional casing would be "Offset". This affects both the GLOBAL_VARS key and the CSS custom property name.
- defaultTransitionOffSet: "default-transition-off-set", + defaultTransitionOffset: "default-transition-offset",Note: This would require updating all references in
AppBar.css.ts,AppScreen.css.ts, andbasicUIPlugin.tsx.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/real-swans-tickle.md(1 hunks).changeset/thirty-melons-invite.md(1 hunks)extensions/plugin-basic-ui/src/basicUIPlugin.css.ts(3 hunks)extensions/plugin-basic-ui/src/basicUIPlugin.tsx(1 hunks)extensions/plugin-basic-ui/src/components/AppBar.css.ts(1 hunks)extensions/plugin-basic-ui/src/components/AppScreen.css.ts(5 hunks)extensions/plugin-basic-ui/src/components/AppScreen.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write source in TypeScript with strict typing enabled across the codebase
Files:
extensions/plugin-basic-ui/src/basicUIPlugin.css.tsextensions/plugin-basic-ui/src/basicUIPlugin.tsxextensions/plugin-basic-ui/src/components/AppBar.css.tsextensions/plugin-basic-ui/src/components/AppScreen.tsxextensions/plugin-basic-ui/src/components/AppScreen.css.ts
extensions/plugin-*/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Plugins must implement only the documented lifecycle hooks (onInit, onBeforePush/onPushed, onBeforePop/onPopped, onBeforeReplace/onReplaced, onBeforeStepPush/onStepPushed, onBeforeStepPop/onStepPopped, onBeforeStepReplace/onStepReplaced, onChanged)
Files:
extensions/plugin-basic-ui/src/basicUIPlugin.css.tsextensions/plugin-basic-ui/src/basicUIPlugin.tsxextensions/plugin-basic-ui/src/components/AppBar.css.tsextensions/plugin-basic-ui/src/components/AppScreen.tsxextensions/plugin-basic-ui/src/components/AppScreen.css.ts
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Include a Changeset entry for any user-facing package change
Files:
.changeset/thirty-melons-invite.md.changeset/real-swans-tickle.md
🧬 Code graph analysis (1)
extensions/plugin-basic-ui/src/components/AppBar.css.ts (2)
extensions/plugin-basic-ui/src/basicUIPlugin.css.ts (2)
globalVars(50-53)android(110-112)extensions/plugin-basic-ui/src/index.ts (1)
globalVars(3-3)
🪛 LanguageTool
.changeset/thirty-melons-invite.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...ic-ui": minor --- Parameterize default appscreen transition offset and appscreen dim hei...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~5-~5: Ensure spelling is correct
Context: ...default appscreen transition offset and appscreen dim height
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Cloudflare Pages
🔇 Additional comments (15)
.changeset/real-swans-tickle.md (1)
1-5: LGTM!The changeset follows the correct format for a patch release. The description is concise and relates to the edge height fix.
.changeset/thirty-melons-invite.md (1)
1-5: LGTM!Minor release is appropriate for adding new configurable options. The description accurately reflects the parameterization changes.
extensions/plugin-basic-ui/src/basicUIPlugin.css.ts (2)
61-64: LGTM!The android values use fixed rem-based offsets appropriate for Android's subtle slide transitions.
99-100: LGTM!The cupertino overrides correctly use percentage-based values for iOS-style full-screen slide transitions.
extensions/plugin-basic-ui/src/components/AppScreen.tsx (1)
238-246: LGTM!Passing
hasAppBarto thecss.appScreenrecipe enables the new CSS variant for conditional top margin and safe-area calculations. This is a clean integration with the CSS-in-TS styling system.extensions/plugin-basic-ui/src/basicUIPlugin.tsx (2)
72-72: LGTM!The
dimHeightoption is correctly wired to the global CSS variable.
79-80: LGTM!The
defaultTransitionOffSetoption is correctly wired to enable runtime configuration of transition offsets.extensions/plugin-basic-ui/src/components/AppBar.css.ts (3)
47-53: LGTM!The Cupertino exit transition now uses the configurable offset variable, enabling consistent parameterization across the UI.
54-62: LGTM!The Android base transform now uses the configurable offset variable, aligning with the parameterization effort.
81-87: Verify: Hardcoded100%in slideInLeft variant.The
slideInLeftvariant still uses a hardcodedtranslate3d(100%, 0, 0)while other transitions now useglobalVars.defaultTransitionOffSet. Is this intentional because slideInLeft requires a full-screen horizontal slide regardless of the configured offset?extensions/plugin-basic-ui/src/components/AppScreen.css.ts (5)
52-78: LGTM!The
hasAppBarvariant centralizes the top margin calculation with proper safe-area inset handling. The@supportsblocks correctly provide fallbacks for both legacyconstant()and modernenv()safe-area functions, ensuring broad browser compatibility.
89-89: LGTM!The dim height is now parameterized via
globalVars.dimHeight, enabling platform-specific values (10rem for Android gradient dim, 100% for Cupertino full overlay).
118-120: LGTM!Cupertino paper transform now uses the configurable transition offset.
127-130: LGTM!Android paper transform now uses the configurable transition offset.
206-211: Good fix for edge height calculation.The edge element now correctly uses
vars.appBar.topMarginwhich includes safe-area insets, instead of onlyglobalVars.appBar.height. This aligns the swipe-back edge with the actual content area, fixing the edge height issue mentioned in the patch changeset.
While aligning edge height with paper heigth