-
Notifications
You must be signed in to change notification settings - Fork 1
feat(ui): create DescriptionList with DescriptionTerm and DescriptionDefinition #1486
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: main
Are you sure you want to change the base?
Changes from all commits
ef78491
0efe2f8
abba480
f8b967d
92b2d7f
312c43a
7aebde5
109aabb
24a86fc
1b69ce8
868b4dd
b7bf791
1c30089
7dab5a8
f8b7ce8
051b951
d1fa1f6
3ade0c2
af033cd
70f4565
5dc49ff
f868c7e
2e858cd
8fc94ef
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,5 @@ | ||
| --- | ||
| "@cloudoperators/juno-ui-components": minor | ||
| --- | ||
|
|
||
| Add `DescriptionList`, `DescriptionTerm` and `DescriptionDefinition` components and export them as `DL`, `DT`, `DD` shorthand. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import React, { ReactNode } from "react" | ||
|
|
||
| const baseStyles = ` | ||
| jn:grid | ||
| jn:items-start | ||
| jn:border-b | ||
| jn:border-theme-default | ||
| jn:bg-dd-background | ||
| jn:text-dd-text | ||
| jn:p-2 | ||
| jn:col-span-3 | ||
| ` | ||
|
|
||
| export interface DescriptionDefinitionProps { | ||
| /** | ||
| * Content to be displayed as the description, accommodating text or more complex nodes to explain or define the associated term. | ||
| */ | ||
| children: ReactNode | ||
| /** | ||
| * Additional class names for applying custom styles or overriding default styles on the <dd> element. | ||
| */ | ||
| className?: string | ||
| } | ||
|
|
||
| /** | ||
| * Represents the definition or description in a description list, rendering as an HTML <dd> element. | ||
| * Pairs with DescriptionTerm to complete the term-description association, offering flexible content styling. | ||
| */ | ||
| export const DescriptionDefinition: React.FC<DescriptionDefinitionProps> = ({ children, className = "" }) => ( | ||
| <dd className={`dd ${baseStyles} ${className}`}>{children}</dd> | ||
| ) | ||
|
|
||
| export const DD = DescriptionDefinition | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import React from "react" | ||
| import { render, screen } from "@testing-library/react" | ||
| import { describe, it, expect } from "vitest" | ||
| import { DescriptionDefinition } from "../DescriptionDefinition" | ||
|
|
||
| describe("DescriptionDefinition", () => { | ||
| it("renders the children correctly", () => { | ||
| render(<DescriptionDefinition>Test Description</DescriptionDefinition>) | ||
| expect(screen.getByText("Test Description")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("applies custom className", () => { | ||
| const customClass = "custom-class" | ||
| render(<DescriptionDefinition className={customClass}>Test Description</DescriptionDefinition>) | ||
| const ddElement = screen.getByText("Test Description") | ||
| expect(ddElement).toHaveClass(customClass) | ||
| }) | ||
|
|
||
| it("renders within a <dd> element", () => { | ||
| render(<DescriptionDefinition>Test Description</DescriptionDefinition>) | ||
| const ddElement = screen.getByText("Test Description") | ||
| expect(ddElement.tagName).toBe("DD") | ||
| }) | ||
|
|
||
| it("can render complex children", () => { | ||
| render( | ||
| <DescriptionDefinition> | ||
| <span>Complex</span> <strong>Content</strong> | ||
| </DescriptionDefinition> | ||
| ) | ||
| expect(screen.getByText("Complex")).toBeInTheDocument() | ||
| expect(screen.getByText("Content")).toBeInTheDocument() | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| export { DescriptionDefinition, type DescriptionDefinitionProps } from "./DescriptionDefinition.component" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import React, { ReactElement } from "react" | ||
| import { DescriptionTermProps } from "../DescriptionTerm" | ||
| import { DescriptionDefinitionProps } from "../DescriptionDefinition" | ||
|
|
||
| export interface DescriptionListProps { | ||
| /** | ||
| * Child components must be either DescriptionTerm or DescriptionDefinition to maintain semantic structure. | ||
| * Supports multiple instances to create a detailed list of terms and definitions. | ||
| */ | ||
| children: | ||
| | ReactElement<DescriptionTermProps | DescriptionDefinitionProps> | ||
| | Array<ReactElement<DescriptionTermProps | DescriptionDefinitionProps>> | ||
| | ReactElement<"div"> | ||
|
Collaborator
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. why allowing a div element?
Member
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. Valid as per spec, so if we offer a component that basically just (re-)creates an html element, we should stick to the spec. Makes total sense if you think about it, it allows for cleanly styling individual pairs of |
||
| /** | ||
| * Determines the alignment of terms within the list. Align terms to the left or right based on preference for display style. | ||
| */ | ||
| alignTerms?: "left" | "right" | ||
| /** | ||
| * Additional custom class names to apply styles to the <dl> element or to extend styling from the design system. | ||
| */ | ||
| className?: string | ||
| } | ||
|
|
||
| /** | ||
| * A wrapper component that semantically represents a list of terms and their corresponding descriptions. | ||
| * This component enforces structure by expecting child elements of DescriptionTerm or DescriptionDefinition, | ||
| * aligning them according to the specified terms alignment. | ||
| */ | ||
| export const DescriptionList: React.FC<DescriptionListProps> = ({ children, alignTerms = "right", className = "" }) => ( | ||
| <dl | ||
| className={`dl jn:grid jn:grid-cols-4 jn:group ${alignTerms === "right" ? "align-right" : "align-left"} ${className}`} | ||
| data-testid="description-list" | ||
| > | ||
| {children} | ||
| </dl> | ||
| ) | ||
|
|
||
| export const DL = DescriptionList | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import React from "react" | ||
| import type { Meta, StoryObj } from "@storybook/react-vite" | ||
| import { DL } from "./DescriptionList.component" | ||
| import { DT } from "../DescriptionTerm/DescriptionTerm.component" | ||
| import { DD } from "../DescriptionDefinition/DescriptionDefinition.component" | ||
|
|
||
| const meta: Meta<typeof DL> = { | ||
| title: "Components/DescriptionList", | ||
| component: DL, | ||
| parameters: { | ||
| docs: { | ||
| description: { | ||
| component: ` | ||
| A wrapper component that semantically represents a list of terms and their corresponding descriptions. | ||
| This component enforces structure by expecting child elements of \`DescriptionTerm\` or \`DescriptionDefinition\`. | ||
| ### Grid Layout | ||
| - By default, the component uses a 4-column grid layout where each \`DescriptionTerm\` spans 1 column and each \`DescriptionDefinition\` spans 3 columns. | ||
| - Customize the grid template by passing other Tailwind CSS grid classes via the \`className\` prop to override the defaults. | ||
| #### Example | ||
| \`\`\`jsx | ||
| <DL className="jn:grid-cols-2"> | ||
| <DT className="jn:col-span-1">Shipping Method</DT> | ||
| <DD className="jn:col-span-2">Standard shipping: 5-7 business days.</DD> | ||
| </DL> | ||
| \`\`\` | ||
| `, | ||
| }, | ||
| }, | ||
| }, | ||
| argTypes: { | ||
| children: { | ||
| control: false, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| export default meta | ||
| type Story = StoryObj<typeof DL> | ||
|
|
||
| export const Default: Story = { | ||
| render: (args) => ( | ||
| <DL {...args}> | ||
| <DT>Shipping</DT> | ||
| <DD>Standard shipping: 5-7 business days.</DD> | ||
| <DT>Payment Options</DT> | ||
| <DD> | ||
| Credit/Debit cards, PayPal, and bank transfer. Lots and lots and lots of options. Oh so many, many options. | ||
| </DD> | ||
| <DT>Delivery Time</DT> | ||
| <DD>1 day, 2 days, 3 days.</DD> | ||
| </DL> | ||
| ), | ||
| } | ||
|
|
||
| export const LeftAligned: Story = { | ||
| render: (args) => ( | ||
| <DL alignTerms="left" {...args}> | ||
| <DT>Shipping Method</DT> | ||
| <DD>Standard shipping: 5-7 business days.</DD> | ||
| <DT>Payment Options</DT> | ||
| <DD> | ||
| Credit/Debit cards, PayPal, and bank transfer. Lots and lots of options available for a seamless transaction | ||
| experience. | ||
| </DD> | ||
| <DT>Delivery Time</DT> | ||
| <DD>Estimated delivery between 1 to 3 business days after shipping.</DD> | ||
| <DT>Return Policy</DT> | ||
| <DD> | ||
| Returns are accepted within 30 days of purchase. Items must be returned in their original packaging and | ||
| condition. | ||
| </DD> | ||
| <DT>Customer Support</DT> | ||
| <DD> | ||
| Available via phone, email, and live chat from 9 AM to 6 PM, Monday to Friday. Our support team is ready to | ||
| assist with any inquiries. | ||
| </DD> | ||
| </DL> | ||
| ), | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import React from "react" | ||
| import { render, screen } from "@testing-library/react" | ||
| import { describe, it, expect } from "vitest" | ||
| import { DescriptionList } from "./DescriptionList.component" | ||
| import { DescriptionTerm } from "../DescriptionTerm" | ||
| import { DescriptionDefinition } from "../DescriptionDefinition" | ||
|
|
||
| describe("DescriptionList", () => { | ||
| it("renders child DescriptionTerm and DescriptionDefinition components correctly", () => { | ||
| render( | ||
| <DescriptionList> | ||
| <DescriptionTerm>Term 1</DescriptionTerm> | ||
| <DescriptionDefinition>Definition 1</DescriptionDefinition> | ||
| </DescriptionList> | ||
| ) | ||
| expect(screen.getByText("Term 1")).toBeInTheDocument() | ||
| expect(screen.getByText("Definition 1")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("applies custom className to the <dl> element", () => { | ||
| const customClass = "custom-class" | ||
| render( | ||
| <DescriptionList className={customClass}> | ||
| <DescriptionTerm>Term 2</DescriptionTerm> | ||
| <DescriptionDefinition>Definition 2</DescriptionDefinition> | ||
| </DescriptionList> | ||
| ) | ||
|
|
||
| const dlElement = screen.getByTestId("description-list") | ||
| expect(dlElement).toHaveClass(customClass) | ||
| }) | ||
|
|
||
| it("aligns terms to the right by default", () => { | ||
| render( | ||
| <DescriptionList> | ||
| <DescriptionTerm>Term 3</DescriptionTerm> | ||
| <DescriptionDefinition>Definition 3</DescriptionDefinition> | ||
| </DescriptionList> | ||
| ) | ||
| }) | ||
|
|
||
| it("aligns terms to the left when specified", () => { | ||
| render( | ||
| <DescriptionList alignTerms="left"> | ||
| <DescriptionTerm>Left Term</DescriptionTerm> | ||
| <DescriptionDefinition>Definition for Left Term</DescriptionDefinition> | ||
| </DescriptionList> | ||
| ) | ||
| }) | ||
|
|
||
| it("renders multiple terms and definitions in a single list", () => { | ||
| render( | ||
| <DescriptionList> | ||
| <DescriptionTerm>Term 4</DescriptionTerm> | ||
| <DescriptionDefinition>Definition 4</DescriptionDefinition> | ||
| <DescriptionTerm>Term 5</DescriptionTerm> | ||
| <DescriptionDefinition>Definition 5</DescriptionDefinition> | ||
| </DescriptionList> | ||
| ) | ||
| expect(screen.getByText("Term 4")).toBeInTheDocument() | ||
| expect(screen.getByText("Definition 4")).toBeInTheDocument() | ||
| expect(screen.getByText("Term 5")).toBeInTheDocument() | ||
| expect(screen.getByText("Definition 5")).toBeInTheDocument() | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| export { DescriptionList, type DescriptionListProps } from "./DescriptionList.component" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import React, { ReactNode } from "react" | ||
|
|
||
| const baseStyles = ` | ||
| jn:grid | ||
| jn:items-start | ||
| jn:border-b | ||
| jn:border-theme-default | ||
| jn:bg-dt-background | ||
| jn:text-dt-text | ||
| jn:font-bold | ||
| jn:gap-y-[0.25rem] | ||
| jn:whitespace-nowrap | ||
| jn:p-2 | ||
| jn:col-span-1 | ||
| jn:group-[.align-right]:text-right | ||
| jn:group-[.align-left]:text-left | ||
| ` | ||
|
|
||
| export interface DescriptionTermProps { | ||
| /** | ||
| * Content to be displayed as the term, which could be simple text or any ReactNode, providing semantic meaning to the associated description. | ||
| */ | ||
| children: ReactNode | ||
| /** | ||
| * Custom class names to apply additional styling to the <dt> element, useful for overrides or custom styles. | ||
| */ | ||
| className?: string | ||
| } | ||
|
|
||
| /** | ||
| * Represents a term in a description list, rendering an HTML <dt> element. | ||
| * Used to denote terms, headers, or keys in a semantic way, allowing for flexible styling. | ||
| */ | ||
| export const DescriptionTerm: React.FC<DescriptionTermProps> = ({ children, className = "" }) => ( | ||
| <dt className={`dt ${baseStyles} ${className}`}>{children}</dt> | ||
| ) | ||
|
|
||
| export const DT = DescriptionTerm |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import React from "react" | ||
| import { render, screen } from "@testing-library/react" | ||
| import { describe, it, expect } from "vitest" | ||
| import { DescriptionTerm } from "./DescriptionTerm.component" | ||
|
|
||
| describe("DescriptionTerm", () => { | ||
| it("renders the children properly", () => { | ||
| render(<DescriptionTerm>Test Term</DescriptionTerm>) | ||
| expect(screen.getByText("Test Term")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("applies custom className to the <dt> element", () => { | ||
| const customClass = "custom-class" | ||
| render(<DescriptionTerm className={customClass}>Styled Term</DescriptionTerm>) | ||
| const dtElement = screen.getByText("Styled Term") | ||
| expect(dtElement).toHaveClass(customClass) | ||
| }) | ||
|
|
||
| it("renders within a <dt> element", () => { | ||
| render(<DescriptionTerm>Term Element</DescriptionTerm>) | ||
| const dtElement = screen.getByText("Term Element") | ||
| expect(dtElement.tagName.toLowerCase()).toBe("dt") | ||
| }) | ||
|
|
||
| it("can render complex children", () => { | ||
| render( | ||
| <DescriptionTerm> | ||
| <span>Complex Term</span> <strong>Content</strong> | ||
| </DescriptionTerm> | ||
| ) | ||
| expect(screen.getByText("Complex Term")).toBeInTheDocument() | ||
| expect(screen.getByText("Content")).toBeInTheDocument() | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Juno contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| export { DescriptionTerm, type DescriptionTermProps } from "./DescriptionTerm.component" |
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.
Do we have a check
<dd>is only valid inside a<dl>?Uh oh!
There was an error while loading. Please reload this page.
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.
Interesting (and somewhat fundamental) question.
We generally do not check for parent validity (we may do in some places, but I can't think of any from the top of my head).
We may check for the validity of children in some places where we do only allow a certain type of children, and then it is not necessarily about strict html validity, but also Juno-UI-Components architectural validity, but even there I doubt we are being 100% consistent tbh.
Short answer: In this case specifically we'd rather not do it., leave as is
The best we could do is display a React warning in the console OR throw an error and prevent the component from rendering. As a general rule we have decided against the latter and preserve some user responsibility. Also, users may choose to render a surrounding
dlelement themselves not using our component, and the result would be totally valid, we couldn't 100% detect that thus still emitting a warning.