feat(plugin-basic-ui): Expose <AppBar> sizing related style customization interfaces #648
Conversation
🦋 Changeset detectedLatest commit: a9b7d21 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: |
a9b7d21
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5ca73574.stackflow-demo.pages.dev |
| Branch Preview URL: | https://expose-appbar-sizing-options.stackflow-demo.pages.dev |
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds five new AppBar global vars (containerPadding, itemGap, fontSize, lineHeight, hitSlop) plus minHeight; populates android defaults and cupertino lineHeight; replaces hard-coded AppBar spacing/typography with those vars and exposes them as AppBar props mapped to CSS variables. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Consumer
participant AppBar as AppBar component
participant RootVars as BasicUI root (assignInlineVars)
participant Styles as AppBar stylesheet
Note over App,AppBar `#D6EAF8`: Props passed (minHeight, containerPadding, itemGap, fontSize, lineHeight, hitSlop)
App->>AppBar: render(props)
AppBar->>RootVars: assignInlineVars(map appBar.* → CSS vars)
RootVars->>Styles: expose CSS variables to stylesheet
Styles->>AppBar: compute layout (padding, gaps, font-size, line-height, hit‑slop)
AppBar-->>App: rendered AppBar with updated layout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 (4)
🚧 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:
🧬 Code graph analysis (1)extensions/plugin-basic-ui/src/components/AppBar.tsx (2)
⏰ 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: |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | a9b7d21 | Commit Preview URL | Nov 25 2025, 02:57 AM |
![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)
extensions/plugin-basic-ui/src/components/AppBar.css.ts (2)
230-251: ConstrainbackButtonTouchAreaExpansionto a single length for safecalc()usageThe pattern of
padding: globalVars.appBar.backButtonTouchAreaExpansion, margin: `calc(-1 * ${globalVars.appBar.backButtonTouchAreaExpansion})`,works well for expanding the hit area while keeping visual bounds unchanged, as long as
backButtonTouchAreaExpansionis a single length (e.g.0.75rem,16px). If a shorthand ("4px 8px") or acalc(...)expression is ever passed in, the resultingcalc(-1 * ...)will be invalid CSS.I’d suggest documenting (and possibly asserting in theme defaults) that
backButtonTouchAreaExpansionmust be a simple length value to avoid hard‑to‑debug styling issues.
291-309:centerMainEdgesizing fromfontSizehas samecalc()assumption as aboveBasing the edge button’s
heightandmaxWidthonglobalVars.appBar.fontSizeis a nice way to keep the tappable region in proportion to the title text. As withbackButtonTouchAreaExpansion, this assumesfontSizeis a plain length (e.g.1rem,20px); if someone configures it as acalc(...)expression,maxWidth: calc(${globalVars.appBar.fontSize} * 5)would become an invalid nestedcalc.Consider documenting that
fontSizeis expected to be a simple length, or adding a brief comment here pointing back to that constraint.
📜 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 (4)
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(7 hunks)extensions/plugin-basic-ui/src/components/AppBar.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- extensions/plugin-basic-ui/src/basicUIPlugin.tsx
- extensions/plugin-basic-ui/src/basicUIPlugin.css.ts
- extensions/plugin-basic-ui/src/components/AppBar.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write source in TypeScript with strict typing enabled across the codebase
Files:
extensions/plugin-basic-ui/src/components/AppBar.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/components/AppBar.css.ts
🧬 Code graph analysis (1)
extensions/plugin-basic-ui/src/components/AppBar.css.ts (2)
extensions/plugin-basic-ui/src/basicUIPlugin.css.ts (2)
globalVars(47-50)cupertino(105-107)extensions/plugin-basic-ui/src/components/AppScreen.css.ts (1)
vars(7-21)
⏰ 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 (4)
extensions/plugin-basic-ui/src/components/AppBar.css.ts (4)
193-208: Android container padding correctly driven bywrappingInlinePaddingUsing
globalVars.appBar.wrappingInlinePaddingfor bothpaddingLeftandpaddingRighton the Android container keeps the outer inline spacing consistent with the new sizing API and looks good.
210-228: Left slot padding now fully theme-driven and coherent across platformsWiring Android’s trailing padding and Cupertino’s leading/trailing paddings to
globalVars.appBar.wrappingInlinePaddingmakes the left slot respect the same configurable inline gutter as the container/right side. Layout-wise this should keep left/center/right alignment consistent as the value changes.
257-289: Center title typography and layout correctly hooked to theme varsMaking
centerMaina flex container and sourcingfontSize/lineHeightfromglobalVars.appBarpluswidthfromvars.appBar.center.mainWidth(for Cupertino) cleanly connects the title’s sizing/width to the new configuration surface. This should make typography and centering respond predictably to theme changes.
322-341: Right slot padding symmetric with left and containerUsing
globalVars.appBar.wrappingInlinePaddingfor Android’s leading padding and both sides on Cupertino keeps the right slot aligned with the left/container gutters and respects the new sizing option. This is consistent with the rest of the AppBar layout.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
extensions/plugin-basic-ui/src/components/AppBar.css.ts (1)
227-248: Hit area and typography vars are wired correctly; fontWeight could be made themeable later
backButton/closeButtonusingpadding: globalVars.appBar.hitSlopwith a matching negativemargin(Lines 238-240) correctly enlarge the touch area without affecting layout flow.centerMainconsumingfontSizeandlineHeight(Lines 257-259), pluscenterTextinheriting them, ensures title sizing is fully driven by the new vars.centerMainEdgebasing itsheightandmaxWidthonfontSize(Lines 295-298) keeps the iOS-style tap target aligned with the visible label.If you expect themes that want a non‑bold title, consider promoting
fontWeight: "bold"(Line 259) to aglobalVars.appBartoken as a future enhancement, but it’s fine as‑is.Also applies to: 254-286, 288-306
📜 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 (4)
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(7 hunks)extensions/plugin-basic-ui/src/components/AppBar.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- extensions/plugin-basic-ui/src/basicUIPlugin.css.ts
- extensions/plugin-basic-ui/src/basicUIPlugin.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write source in TypeScript with strict typing enabled across the codebase
Files:
extensions/plugin-basic-ui/src/components/AppBar.css.tsextensions/plugin-basic-ui/src/components/AppBar.tsx
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/components/AppBar.css.tsextensions/plugin-basic-ui/src/components/AppBar.tsx
🧬 Code graph analysis (1)
extensions/plugin-basic-ui/src/components/AppBar.tsx (2)
extensions/plugin-basic-ui/src/basicUIPlugin.css.ts (1)
globalVars(48-51)extensions/plugin-basic-ui/src/index.ts (1)
globalVars(3-3)
⏰ 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 (2)
extensions/plugin-basic-ui/src/components/AppBar.tsx (1)
19-42: AppBar sizing props are consistently threaded from type to CSS variablesThe newly added sizing props (
minHeight,wrappingInlinePadding,fontSize,lineHeight,hitSlop,surroundingContentSpacing) are correctly:
- included in
AppBarPropsviaPick<GlobalVars["appBar"], ...>(Line 36-41),- destructured from the component props (Line 102-107),
- and mapped into the corresponding
globalVars.appBar.*CSS vars viaassignInlineVarswithcompactMap(Line 338-344),which keeps the runtime behavior aligned with the theme contract in
basicUIPlugin.css.ts. I don’t see any typing or wiring gaps here.Also applies to: 102-107, 338-344
extensions/plugin-basic-ui/src/components/AppBar.css.ts (1)
193-208: Spacing tokens for padding vs gaps look coherent across themes
wrappingInlinePaddingis now used for the outer container padding (containeron Android,left/rightCupertino side paddings), whilesurroundingContentSpacingis used as the horizontal gap between edge content and the title (left/rightpadding on Lines 215 and 325). This gives a clear, symmetric separation between “outer padding” and “intra-content spacing” on both Android and Cupertino without any obvious layout regressions in the code.Also applies to: 210-225, 319-335
This PR allows users to customize
containerPaddingitemGapfontSizehitSlop