-
Notifications
You must be signed in to change notification settings - Fork 1
feat: rework PlanDisplay to Plans 2.0 design #170
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
base: master
Are you sure you want to change the base?
Changes from all commits
f5bc5bf
1449df8
ccf8e58
28e879e
1ac5e4b
c18d5de
99f3b90
0a2d87e
d9931e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| releases: | ||
| "@nimbus-ds/plan-display": minor | ||
| "@nimbus-ds/patterns-webpack": patch | ||
|
|
||
| declined: | ||
| - nimbus-patterns | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,16 @@ | ||
| # Changelog | ||
|
|
||
| EmptyApp allows the user to build marketing-style landing pages for apps. It features internal components meant to build hero sections, content sections with images and text with features, and payment plans. | ||
| PlanDisplay allows the user to present subscription plans and pricing. It features internal components to build plan cards with headers, prices, feature bullets, and footers β including featured-plan and gradient variants. | ||
|
|
||
| ## 2026-06-11 `2.0.0` | ||
|
|
||
| ### β¨ Features | ||
|
|
||
| - Reworked `PlanDisplay` to the Plans 2.0 design as the canonical layout, matching the Figma master component. `PlanDisplay.Card` now renders the default card with a level-2 shadow, plus two opt-in variants: `ribbonLabel` (a "MΓ‘s escogido" ribbon with a primary 2px border) and `gradient` (primary-surface β background gradient). Added a `tag` slot on `PlanDisplay.Header` (e.g. "Plan actual"), a new `PlanDisplay.Price` subcomponent (current price + optional previous price, period and annual note), a `badge` slot on `PlanDisplay.Bullet` (e.g. "Nuevo"), and an `icon` slot on `PlanDisplay.Footer` for add-on rows. (by [@noecondoleo](https://github.com/noecondoleo)) | ||
|
|
||
| ### β οΈ Breaking changes | ||
|
|
||
| - Removed the legacy `highlighted` prop on `PlanDisplay.Card`. Use `ribbonLabel` for the featured plan or `gradient` for the gradient variant. (by [@noecondoleo](https://github.com/noecondoleo)) | ||
|
Comment on lines
+7
to
+13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Maintainability & Code Quality | π Major | β‘ Quick win Fix CHANGELOG format to match repository contract (headers + PR references). Line 7β13 uses non-standard headers and omits mandatory PR references in entries. Please switch to the required category names/emojis and include As per coding guidelines, " π€ Prompt for AI AgentsSources: Coding guidelines, Path instructions
Comment on lines
+7
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Formatting deviates from
Suggested fix: #### π New features
- Reworked `PlanDisplay` to the Plans 2.0 design... ([#170](https://github.com/TiendaNube/nimbus-patterns/pull/170) by [@noecondoleo](https://github.com/noecondoleo))
#### π Breaking changes
- Removed the legacy `highlighted` prop on `PlanDisplay.Card`... ([#170](https://github.com/TiendaNube/nimbus-patterns/pull/170) by [@noecondoleo](https://github.com/noecondoleo)) |
||
|
|
||
| ## 2026-01-15 `1.1.1` | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import { | |
| PlanDisplayCardContent, | ||
| PlanDisplayCardFooter, | ||
| PlanDisplayCardHeader, | ||
| PlanDisplayCardPrice, | ||
| PlanDisplayCardSpacing, | ||
| } from "./components"; | ||
|
|
||
|
|
@@ -23,7 +24,7 @@ const PlanDisplay: React.FC<PlanDisplayProps> & PlanDisplayComponents = ({ | |
| gridTemplateColumns={`repeat(auto-fit, minmax(${minPlanWidth}, 1fr))`} | ||
| gap="6" | ||
| justifyContent="center" | ||
| alignItems="flex-start" | ||
| alignItems="stretch" | ||
| {...rest} | ||
| > | ||
| {children} | ||
|
|
@@ -32,6 +33,7 @@ const PlanDisplay: React.FC<PlanDisplayProps> & PlanDisplayComponents = ({ | |
|
|
||
| PlanDisplay.Card = PlanDisplayCard; | ||
| PlanDisplay.Header = PlanDisplayCardHeader; | ||
| PlanDisplay.Price = PlanDisplayCardPrice; | ||
| PlanDisplay.Content = PlanDisplayCardContent; | ||
| PlanDisplay.Spacing = PlanDisplayCardSpacing; | ||
| PlanDisplay.Bullet = PlanDisplayBullet; | ||
|
|
@@ -41,6 +43,7 @@ PlanDisplay.displayName = "PlanDisplay"; | |
| PlanDisplay.Card.displayName = "PlanDisplay.Card"; | ||
| PlanDisplay.Bullet.displayName = "PlanDisplay.Bullet"; | ||
| PlanDisplay.Header.displayName = "PlanDisplay.CardHeader"; | ||
| PlanDisplay.Price.displayName = "PlanDisplay.Price"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Every other subcomponent here sets its public I confirmed this by running and the resulting Also, Fix: rename this to |
||
| PlanDisplay.Content.displayName = "PlanDisplay.CardContent"; | ||
| PlanDisplay.Spacing.displayName = "PlanDisplay.CardSpacing"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,25 +5,45 @@ import { PlanDisplayBulletProps } from "./planDisplayBullet.types"; | |
| const PlanDisplayBullet: React.FC<PlanDisplayBulletProps> = ({ | ||
| icon, | ||
| disabled, | ||
| badge, | ||
| children, | ||
| }) => ( | ||
| <Box display="flex" gap="2"> | ||
| <Box display="flex" gap="2" alignItems="center"> | ||
| <Box display="flex" alignItems="center"> | ||
| <Text | ||
| as="span" | ||
| color={disabled ? "neutral-interactive" : "success-interactive"} | ||
| color={disabled ? "neutral-interactive" : "primary-interactive"} | ||
| > | ||
| <Box display="flex" alignItems="center"> | ||
| {icon} | ||
| </Box> | ||
|
Comment on lines
14
to
19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π― Functional Correctness | π‘ Minor | β‘ Quick win Avoid nesting a default On Line 17, π€ Prompt for AI Agents |
||
| </Text> | ||
| </Box> | ||
| <Text | ||
| fontWeight="medium" | ||
| color={disabled ? "neutral-interactive" : "neutral-textLow"} | ||
| > | ||
| {children} | ||
| </Text> | ||
| {badge ? ( | ||
| <Text | ||
| fontWeight="regular" | ||
| color={disabled ? "neutral-interactive" : "neutral-textLow"} | ||
| > | ||
| {children}{" "} | ||
| {/* Native span: Box drops `style`, so `vertical-align` would never apply on a Box. */} | ||
| <span | ||
| style={{ | ||
| display: "inline-flex", | ||
| gap: "var(--nimbus-spacing-2)", | ||
| verticalAlign: "middle", | ||
| }} | ||
| > | ||
| {badge} | ||
| </span> | ||
| </Text> | ||
| ) : ( | ||
| <Text | ||
| fontWeight="regular" | ||
| color={disabled ? "neutral-interactive" : "neutral-textLow"} | ||
| > | ||
| {children} | ||
| </Text> | ||
| )} | ||
| </Box> | ||
| ); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,11 @@ import { PropsWithChildren, ReactNode } from "react"; | |
| export type PlanDisplayBulletProperties = { | ||
| icon: ReactNode; | ||
| disabled?: boolean; | ||
| /** | ||
| * Optional slot rendered inline after the bullet text (e.g. a `<Tag>` like "Nuevo"). | ||
| * @TJS-type React.ReactNode | ||
| */ | ||
| badge?: ReactNode; | ||
|
Comment on lines
3
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Maintainability & Code Quality | π Major | β‘ Quick win Use an interface for
As per coding guidelines, " π€ Prompt for AI AgentsSources: Coding guidelines, Path instructions |
||
| }; | ||
| export type PlanDisplayBulletProps = | ||
| PropsWithChildren<PlanDisplayBulletProperties>; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,78 @@ | ||
| import React from "react"; | ||
| import { Box, BoxProps, Card } from "@nimbus-ds/components"; | ||
| import { Box, Text } from "@nimbus-ds/components"; | ||
| import { PlanDisplayCardProps } from "./planDisplayCard.types"; | ||
|
|
||
| const GRADIENT_BACKGROUND = | ||
| "linear-gradient(194.55deg, var(--nimbus-colors-primary-surface) 4.18%, var(--nimbus-colors-neutral-background) 45.97%)"; | ||
|
|
||
| const PlanDisplayCard: React.FC<PlanDisplayCardProps> = ({ | ||
| highlighted, | ||
| ribbonLabel, | ||
| gradient, | ||
| children, | ||
| style, | ||
| ...rest | ||
| }) => { | ||
| const highlightedProps: BoxProps = { | ||
| borderColor: "primary-interactive", | ||
| borderRadius: "2", | ||
| borderStyle: "solid", | ||
| borderWidth: "3", | ||
| boxShadow: "3", | ||
| }; | ||
| const hasRibbon = Boolean(ribbonLabel); | ||
|
|
||
| return ( | ||
| <Box {...(highlighted ? highlightedProps : {})} {...rest}> | ||
| <Card>{children}</Card> | ||
| <Box {...rest} display="flex" flexDirection="column" height="100%"> | ||
| {/* Ribbon area: visible when `ribbonLabel` is set, an invisible placeholder of the | ||
| same height otherwise. Rendering it on every card reserves identical top space so | ||
| the content rows (subtitle, price, bullets) align across the grid. */} | ||
| <div | ||
| aria-hidden={hasRibbon ? undefined : true} | ||
| style={{ | ||
| display: "flex", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| backgroundColor: hasRibbon | ||
| ? "var(--nimbus-colors-primary-interactive)" | ||
| : "transparent", | ||
| paddingTop: "var(--nimbus-spacing-0-5)", | ||
| paddingBottom: "var(--nimbus-spacing-2)", | ||
| borderTopLeftRadius: "var(--nimbus-shape-border-radius-2)", | ||
| borderTopRightRadius: "var(--nimbus-shape-border-radius-2)", | ||
| marginBottom: "calc(var(--nimbus-spacing-2) * -1)", | ||
| }} | ||
| > | ||
| <Text fontWeight="medium" color="neutral-background" textAlign="center"> | ||
| {ribbonLabel || "Β "} | ||
| </Text> | ||
| </div> | ||
| {/* Content grows to fill the card height (cards in a row stretch to match), so footers | ||
| anchored with `margin-top: auto` line up at the bottom. */} | ||
| <Box | ||
| flex="1 1 auto" | ||
| display="flex" | ||
| flexDirection="column" | ||
| backgroundColor="neutral-background" | ||
| borderRadius="2" | ||
| overflow="hidden" | ||
| {...(hasRibbon | ||
| ? { | ||
| borderColor: "primary-interactive", | ||
| borderStyle: "solid", | ||
| borderWidth: "2", | ||
| } | ||
| : { boxShadow: "2" })} | ||
| > | ||
| {/* Content lives in a native element because @nimbus-ds/components Box drops the | ||
| `style` prop (it renders only sprinkle-generated styles). Routing the gradient | ||
| background and any consumer `style` through here is the only way they take effect. | ||
| The div fills the card as a flex column so footers still anchor with `margin-top: auto`. */} | ||
| <div | ||
| style={{ | ||
| display: "flex", | ||
| flexDirection: "column", | ||
| flex: "1 1 auto", | ||
| padding: "var(--nimbus-spacing-4)", | ||
| background: gradient && !hasRibbon ? GRADIENT_BACKGROUND : undefined, | ||
| ...style, | ||
| }} | ||
| > | ||
| {children} | ||
| </div> | ||
| </Box> | ||
| </Box> | ||
|
Comment on lines
+18
to
76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This drops the I verified the stated justification in the comment on lines 59-62 β That said, worth reconsidering before merging:
|
||
| ); | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,17 @@ describe("GIVEN <PlanDisplayCard />", () => { | |
| expect(screen.getByText("Body content")).toBeDefined(); | ||
| }); | ||
|
|
||
| it("SHOULD render highlighted state correctly", () => { | ||
| makeSut({ highlighted: true }); | ||
| it("SHOULD render the gradient variant correctly", () => { | ||
| makeSut({ gradient: true }); | ||
|
|
||
| expect(screen.getByText("Body content")).toBeDefined(); | ||
| }); | ||
|
|
||
| it("SHOULD render the ribbon label when provided", () => { | ||
| makeSut({ ribbonLabel: "MΓ‘s escogido" }); | ||
|
|
||
| expect(screen.getByText("MΓ‘s escogido")).toBeDefined(); | ||
| expect(screen.getByText("Body content")).toBeDefined(); | ||
| }); | ||
|
Comment on lines
+21
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Maintainability & Code Quality | π΅ Trivial | β‘ Quick win Add a precedence test for The public contract documents that π€ Prompt for AI Agents |
||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,32 @@ | ||
| import React from "react"; | ||
| import { Box } from "@nimbus-ds/components"; | ||
| import { Box, Divider, Text } from "@nimbus-ds/components"; | ||
| import { PlanDisplayCardFooterProperties } from "./planDisplayCardFooter.types"; | ||
| import PlanDisplayCardSpacing from "../PlanDisplayCardSpacing"; | ||
|
|
||
| const PlanDisplayCardFooter: React.FC<PlanDisplayCardFooterProperties> = ({ | ||
| icon, | ||
| children, | ||
| }) => ( | ||
| <Box display="flex" flexDirection="column" gap="4"> | ||
| <PlanDisplayCardSpacing /> | ||
| // `marginTop: auto` pins the footer to the bottom of the (full-height, flex-column) card | ||
| // so footers stay aligned across cards in the same row. Native element on purpose: Box | ||
| // discards `style`, so the `margin-top: auto` would be a no-op on a Box. | ||
| <div style={{ display: "flex", flexDirection: "column", marginTop: "auto" }}> | ||
| <Box mt="3" mb="3"> | ||
| <Divider /> | ||
| </Box> | ||
|
|
||
| <Box>{children}</Box> | ||
| </Box> | ||
| {icon ? ( | ||
| <Box display="flex" gap="2" alignItems="flex-start"> | ||
| <Text as="span" color="primary-interactive"> | ||
| <Box display="flex" alignItems="center"> | ||
| {icon} | ||
| </Box> | ||
| </Text> | ||
| <Box>{children}</Box> | ||
| </Box> | ||
| ) : ( | ||
| <Box>{children}</Box> | ||
| )} | ||
| </div> | ||
|
Comment on lines
+12
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same native- |
||
| ); | ||
|
|
||
| export { PlanDisplayCardFooter }; | ||
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.
This should be aβ οΈ Breaking change" and versions as
majorrelease, notminor. This PR removes the publichighlightedprop fromPlanDisplay.Card(seeplanDisplayCard.types.ts), which the CHANGELOG in this same PR correctly labels a "π /2.0.0. Aminorrelease entry here meansyarn bump:stablewill actually bump@nimbus-ds/plan-displayto1.2.0, not2.0.0β contradicting the CHANGELOG and shipping a breaking API removal under a non-major version, which silently breaks consumers pinned with^1.x.This is also exactly the case the repo's own
.coderabbit.yamlcustom_checks.Unversioned Breaking Changesis designed to catch: "All breaking changes MUST have a MAJOR version bump (x.0.0)."Please change this to: