From 15296cc6581346a11632ceb7cad3c423d9875419 Mon Sep 17 00:00:00 2001 From: Haider Alshamma Date: Fri, 13 Dec 2024 15:09:34 -0500 Subject: [PATCH] fix: layout, visual and API issues with the DescriptionList --- src/DescriptionList/DescriptionDetails.tsx | 45 ++++++++++-- src/DescriptionList/DescriptionList.story.tsx | 69 ++++++++++--------- src/DescriptionList/DescriptionList.tsx | 6 +- .../DescriptionListContext.tsx | 1 + src/DescriptionList/DescriptionTerm.tsx | 46 +++++++++++-- src/DescriptionList/README.md | 6 +- 6 files changed, 124 insertions(+), 49 deletions(-) diff --git a/src/DescriptionList/DescriptionDetails.tsx b/src/DescriptionList/DescriptionDetails.tsx index 6cc6c0afd..25eed751a 100644 --- a/src/DescriptionList/DescriptionDetails.tsx +++ b/src/DescriptionList/DescriptionDetails.tsx @@ -5,22 +5,39 @@ export const DescriptionDetails = styled.dd(({ theme }) => { const { autoLayoutBreakpoint, showDivider, density, layout } = useDescriptionListContext(); return { + // narrow stuff + non-auto layout margin: 0, + color: theme.colors.black, paddingLeft: theme.space.none, paddingRight: theme.space.none, - color: theme.colors.darkGrey, + + ...(layout !== "inline" && { + paddingTop: theme.space.none, + }), ...(density === "compact" && { - paddingTop: theme.space.x0_25, paddingBottom: theme.space.x0_25, + + ...(layout === "inline" && { + paddingTop: theme.space.x0_25, + }), }), + ...(density === "medium" && { - paddingTop: theme.space.x0_75, paddingBottom: theme.space.x0_75, + + ...(layout === "inline" && { + paddingTop: theme.space.x0_75, + }), }), + ...(density === "relaxed" && { - paddingTop: theme.space.x1_5, paddingBottom: theme.space.x1_5, + paddingTop: theme.space.x0_25, + + ...(layout === "inline" && { + paddingTop: theme.space.x1_5, + }), }), ...(showDivider && @@ -30,7 +47,12 @@ export const DescriptionDetails = styled.dd(({ theme }) => { borderTopColor: theme.colors.lightGrey, }), + "&:nth-child(2)": { + border: "none", + }, + [`@container (min-width: ${autoLayoutBreakpoint})`]: { + // wide auto layout ...(showDivider && layout !== "stacked" && { borderTopWidth: "1px", @@ -38,9 +60,18 @@ export const DescriptionDetails = styled.dd(({ theme }) => { borderTopColor: theme.colors.lightGrey, }), - "&:nth-child(2)": { - border: "none", - }, + ...(density === "compact" && + layout !== "stacked" && { + paddingTop: theme.space.x0_25, + }), + ...(density === "medium" && + layout !== "stacked" && { + paddingTop: theme.space.x0_75, + }), + ...(density === "relaxed" && + layout !== "stacked" && { + paddingTop: theme.space.x1_5, + }), }, }; }); diff --git a/src/DescriptionList/DescriptionList.story.tsx b/src/DescriptionList/DescriptionList.story.tsx index 983c9c71a..b5e465096 100644 --- a/src/DescriptionList/DescriptionList.story.tsx +++ b/src/DescriptionList/DescriptionList.story.tsx @@ -1,6 +1,6 @@ import React from "react"; import { Flex } from "../Flex"; -import { Heading4 } from "../Type"; +import { Heading4, Text } from "../Type"; import { Link } from "../Link"; import { Icon } from "../Icon"; import { StatusIndicator } from "../StatusIndicator"; @@ -16,9 +16,10 @@ const SampleContent = () => ( Customer Nulogy - - Order number - + + Order number + + P12-90381-2039 @@ -38,37 +39,34 @@ const SampleContent = () => ( ); -export function WithDifferentLayouts() { +export function Layout() { return ( - + Auto Layout (Default) - - - Stacked Layout - - - - - - - Inline Layout - - - - + + Stacked Layout + + + + + + Inline Layout + + + ); } -export function WithDifferentDensities() { +export function Density() { return ( - + Compact Density @@ -108,9 +106,9 @@ export function WithDifferentDensities() { ); } -export function WithDifferentFontSizes() { +export function FontSize() { return ( - + Smaller Font Size @@ -135,9 +133,9 @@ export function WithDifferentFontSizes() { ); } -export function WithDividers() { +export function Divider() { return ( - + With Dividers (Stacked layout) @@ -154,9 +152,9 @@ export function WithDividers() { ); } -export function WithCustomBreakpoints() { +export function AutoLayoutCustomBreakpoint() { return ( - + Auto layout custom breakpoint (800px) @@ -167,9 +165,9 @@ export function WithCustomBreakpoints() { ); } -export function WithCustomDescriptionTermWidth() { +export function CustomDescriptionTermWidth() { return ( - + Custom description term max-width (33.33%) @@ -194,13 +192,12 @@ export function WithCustomDescriptionTermWidth() { export function CombinedFeatures() { return ( - + - Auto Layout + Dividers + Compact Density + Small Font + 40% Term Width + 720px auto layout breakpoint + Auto Layout, divider, compact density, small font, 40% term width, 720px auto layout breakpoint + + Stacked, no dividers, relaxed, large font + + + + ); } diff --git a/src/DescriptionList/DescriptionList.tsx b/src/DescriptionList/DescriptionList.tsx index 3ce691362..c3bf7b7da 100644 --- a/src/DescriptionList/DescriptionList.tsx +++ b/src/DescriptionList/DescriptionList.tsx @@ -11,6 +11,7 @@ export function DescriptionList({ showDivider = false, density = "medium", fontSize = "medium", + lineHeight = "base", autoLayoutBreakpoint = "640px", children, }: Props) { @@ -21,6 +22,7 @@ export function DescriptionList({ showDivider={showDivider} density={density} fontSize={fontSize} + lineHeight={lineHeight} autoLayoutBreakpoint={autoLayoutBreakpoint} > @@ -36,13 +38,13 @@ export const DescriptionListContainer = styled.div({ }); export const StyledDescriptionList = styled.dl(({ theme }) => { - const { descriptionTermMaxWidth, layout, fontSize, autoLayoutBreakpoint } = useDescriptionListContext(); + const { descriptionTermMaxWidth, layout, fontSize, lineHeight, autoLayoutBreakpoint } = useDescriptionListContext(); return { margin: 0, display: "grid", fontSize: theme.fontSizes[fontSize] ?? theme.fontSizes.medium, - lineHeight: theme.lineHeights.base, + lineHeight: theme.lineHeights[lineHeight] ?? theme.lineHeights.base, ...(layout === "inline" && { gridTemplateColumns: `minmax(0, ${descriptionTermMaxWidth}) auto`, diff --git a/src/DescriptionList/DescriptionListContext.tsx b/src/DescriptionList/DescriptionListContext.tsx index fb9cd3f5c..9b67947e1 100644 --- a/src/DescriptionList/DescriptionListContext.tsx +++ b/src/DescriptionList/DescriptionListContext.tsx @@ -7,6 +7,7 @@ export interface DescriptionListConfig { showDivider?: boolean; density?: "medium" | "compact" | "relaxed"; fontSize?: keyof DefaultNDSThemeType["fontSizes"]; + lineHeight?: keyof DefaultNDSThemeType["lineHeights"]; autoLayoutBreakpoint?: string; } diff --git a/src/DescriptionList/DescriptionTerm.tsx b/src/DescriptionList/DescriptionTerm.tsx index 2a167c506..88213b4be 100644 --- a/src/DescriptionList/DescriptionTerm.tsx +++ b/src/DescriptionList/DescriptionTerm.tsx @@ -2,27 +2,42 @@ import { styled } from "styled-components"; import { useDescriptionListContext } from "./DescriptionListContext"; export const DescriptionTerm = styled.dt(({ theme }) => { - const { showDivider, density, layout } = useDescriptionListContext(); + const { showDivider, density, layout, autoLayoutBreakpoint } = useDescriptionListContext(); return { + // narrow stuff + non-auto layout gridColumnStart: 1, color: theme.colors.midGrey, paddingLeft: theme.space.none, paddingRight: theme.space.none, + ...(layout !== "inline" && { + paddingBottom: theme.space.none, + }), + ...(density === "compact" && { paddingTop: theme.space.x0_25, - paddingBottom: theme.space.x0_25, + + ...(layout === "inline" && { + paddingBottom: theme.space.x0_25, + }), }), ...(density === "medium" && { paddingTop: theme.space.x0_75, - paddingBottom: theme.space.x0_75, + + ...(layout === "inline" && { + paddingBottom: theme.space.x0_75, + }), }), ...(density === "relaxed" && { paddingTop: theme.space.x1_5, - paddingBottom: theme.space.x1_5, + paddingBottom: theme.space.x0_25, + + ...(layout === "inline" && { + paddingBottom: theme.space.x1_5, + }), }), ...(showDivider && { @@ -34,5 +49,28 @@ export const DescriptionTerm = styled.dt(({ theme }) => { "&:first-child": { border: "none", }, + + [`@container (min-width: ${autoLayoutBreakpoint})`]: { + // wide auto layout + ...(showDivider && + layout !== "stacked" && { + borderTopWidth: "1px", + borderTopStyle: "solid", + borderTopColor: theme.colors.lightGrey, + }), + + ...(density === "compact" && + layout !== "stacked" && { + paddingBottom: theme.space.x0_25, + }), + ...(density === "medium" && + layout !== "stacked" && { + paddingBottom: theme.space.x0_75, + }), + ...(density === "relaxed" && + layout !== "stacked" && { + paddingBottom: theme.space.x1_5, + }), + }, }; }); diff --git a/src/DescriptionList/README.md b/src/DescriptionList/README.md index 89bdb74b4..11601b5ae 100644 --- a/src/DescriptionList/README.md +++ b/src/DescriptionList/README.md @@ -1,7 +1,6 @@ # DescriptionList -The DescriptionList component is a structured list that pairs terms with their descriptions, providing a clear way to display key-value information. -The DescriptionList is ideal for displaying key-value pair information in a structured format organizing and explaining related information. +The DescriptionList component is a structured list that pairs terms with their descriptions, providing a standardized way to display key-value pair information in a structured format organizing and explaining related information. ## Basic Usage @@ -27,10 +26,11 @@ function MyComponent() { | `layout` | "stacked" \| "inline" \| "auto" | Controls the arrangement of terms and details | `"auto"` | | `density` | "compact" \| "medium" \| "relaxed" | Controls the spacing between items | `"medium"` | | `fontSize` | `keyof FontSizes` | Controls the text size | `"medium"` | +| `lineHeight` | `keyof LineHeights` | Controls the text line height | `"base"` | | `showDivider` | boolean | Shows divider lines between items | `false` | | `descriptionTermMaxWidth` | string | Controls the maximum width of description terms | `"50%"` | | `autoLayoutBreakpoint` | string | Defines the container width at which the layout changes when layout is set to `auto` | `"640px"` | -| `children` | ReactNode | Content to be displayed inside the list | - | +| `children` | ReactNode | `DescriptionTerm` and `DescriptionDetails` to be placed inside the list | - | ## Accessibility