Skip to content
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

NavigationTabs Styling #2508

Open
wants to merge 45 commits into
base: feature/tabs
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
fbd5b24
[nav-tabs-style] Add styles/style props for custom styles
beaesguerra Mar 17, 2025
62b657d
[nav-tabs-style] Use boxshadow styling for underline instead of ::aft…
beaesguerra Mar 17, 2025
1a8410b
[nav-tabs-style] Add current styling
beaesguerra Mar 17, 2025
808642b
[nav-tabs-style] Add press styling
beaesguerra Mar 18, 2025
dda6285
[nav-tabs-style] adjust spacing based on media query
beaesguerra Mar 18, 2025
4d233ce
[nav-tabs-style] Add rtl option to AllVariants
beaesguerra Mar 18, 2025
fa9178e
Revert "[nav-tabs-style] Add rtl option to AllVariants"
beaesguerra Mar 18, 2025
4bca67f
[nav-tabs-style] add file for string constants for testing
beaesguerra Mar 18, 2025
ba21c38
[nav-tabs-style] Add RTL case in stories
beaesguerra Mar 18, 2025
af4523a
[nav-tabs-style] set current in nav tabs stories
beaesguerra Mar 18, 2025
a40844e
[nav-tabs-style] Add ZoomWrapper
beaesguerra Mar 18, 2025
88e86c7
[nav-tabs-style] Add Zoom stories
beaesguerra Mar 18, 2025
aba7023
[nav-tabs-style] Make AllVariants responsive by rendering a list inst…
beaesguerra Mar 18, 2025
68c6f33
[nav-tabs-style] make AllVariants a list in Zoom stories
beaesguerra Mar 18, 2025
b1e8976
[nav-tabs-style] Allow navigation tabs to be horizontally scrollable
beaesguerra Mar 18, 2025
312f738
[nav-tabs-style] setting up examples with long text
beaesguerra Mar 19, 2025
7e64d30
[nav-tabs-style] Prevent wrapping
beaesguerra Mar 19, 2025
cc70d83
[nav-tabs-style] scenarioslayout
beaesguerra Mar 19, 2025
91c6aa9
[nav-tabs-style] address icon only
beaesguerra Mar 19, 2025
d4db4b0
[nav-tabs-style] make sure native anchor tag is styled properly
beaesguerra Mar 19, 2025
ed9f866
[nav-tabs-style] make focus outline consistent on link child. using b…
beaesguerra Mar 19, 2025
cf0dc05
[nav-tabs-style] rtl
beaesguerra Mar 19, 2025
30b3839
[nav-tabs-style] native anchor tag examples
beaesguerra Mar 19, 2025
fd54ec2
[nav-tabs-style] make nav scrollable horizontally if there are too ma…
beaesguerra Mar 19, 2025
67146d5
[nav-tabs-style] Add example within header
beaesguerra Mar 19, 2025
9f6e16b
[nav-tabs-style] Configure Language direction in Storybook toolbar
beaesguerra Mar 19, 2025
27c3540
[nav-tabs-style] address a11y errors
beaesguerra Mar 19, 2025
ca44911
[nav-tabs-style] fix aria-label
beaesguerra Mar 19, 2025
cd941b2
[nav-tabs-style] update semantic token use
beaesguerra Mar 19, 2025
b1f1d38
[nav-tabs-style] Improve docs
beaesguerra Mar 19, 2025
1031f0a
[nav-tabs-style] cleanup
beaesguerra Mar 19, 2025
d2f791e
[nav-tabs-style] cleanup
beaesguerra Mar 19, 2025
8a7b2a5
[nav-tabs-style] clean up
beaesguerra Mar 20, 2025
4dfe2b7
[nav-tabs-style] use decorator instead of zoomwrapper
beaesguerra Mar 20, 2025
b21c0cb
[nav-tabs-style] cleanup
beaesguerra Mar 20, 2025
6f93797
[nav-tabs-style] docs(changeset): NavigationTabs: Update styles to ha…
beaesguerra Mar 20, 2025
9f8b37e
[nav-tabs-style] cleanup
beaesguerra Mar 20, 2025
0cdf1bd
[nav-tabs-style] disable snapshots
beaesguerra Mar 20, 2025
b8c8ad6
[nav-tabs-style] address code review comments. typos, docs
beaesguerra Mar 21, 2025
476de45
[nav-tabs-style] make hover state underline thinner and make sure cur…
beaesguerra Mar 21, 2025
b430e63
[nav-tabs-style] make custom style more obvious
beaesguerra Mar 21, 2025
4cef4a4
[nav-tabs-style] try setting viewports to address cropping zoom stories
beaesguerra Mar 21, 2025
296441b
[nav-tabs-style] try setting default viewport instead
beaesguerra Mar 21, 2025
28d53d3
[nav-tabs-style] Try setting width and max width
beaesguerra Mar 21, 2025
9c4fa3d
[nav-tabs-style] go back to disabling snapshot for zoom
beaesguerra Mar 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/twenty-actors-tease.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-tabs": minor
---

NavigationTabs: Update styles to handle different scenarios
77 changes: 76 additions & 1 deletion .storybook/preview.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as React from "react";

Check warning on line 1 in .storybook/preview.tsx

View workflow job for this annotation

GitHub Actions / Lint / Lint (ubuntu-latest, 20.x)

File ignored by default.

Check warning on line 1 in .storybook/preview.tsx

View workflow job for this annotation

GitHub Actions / Lint / Lint (ubuntu-latest, 20.x)

File ignored by default.
import wonderBlocksTheme from "./wonder-blocks-theme";
import {Decorator} from "@storybook/react";
import {semanticColor} from "@khanacademy/wonder-blocks-tokens";
Expand Down Expand Up @@ -115,9 +115,42 @@
);
};

/**
* Wraps a story with `<div dir="rtl">` so it is shown in rtl mode.
*/
const withLanguageDirection: Decorator = (Story, context) => {
if (context.globals.direction === "rtl") {
return (
<div dir="rtl">
<Story />
</div>
)
} else {
return <Story />
}
}

/**
* Wraps a story with styling that simulates [zoom](https://developer.mozilla.org/en-US/docs/Web/CSS/zoom).
*
* Note: It is still important to test with real browser zoom in different
* browsers. For example, using the CSS zoom property in Safari looks a bit
* different than when you zoom in the browser.
*/
const withZoom: Decorator = (Story, context) => {
if (context.globals.zoom) {
return (
<div style={{ zoom: context.globals.zoom }}>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to find a way for us to easily configure a story to be a specific zoom level so we can have some Chromatic tests covering those cases too. I came across the CSS zoom property to control magnification

<Story />
</div>
)
}
return <Story />
}

const preview: Preview = {
parameters,
decorators: [withThemeSwitcher],
decorators: [withThemeSwitcher, withLanguageDirection, withZoom],
globalTypes: {
// Allow the user to select a theme from the toolbar.
theme: {
Expand All @@ -143,6 +176,48 @@
dynamicTitle: true,
},
},
direction: {
description: "The language direction to use",
toolbar: {
title: "Language Direction",
icon: "globe",
items: [
{
value: "ltr",
icon: "arrowrightalt",
title: "Left to Right",
},
{
value: "rtl",
icon: "arrowleftalt",
title: "Right to Left",
},
],
dynamicTitle: true,
},
},
zoom: {
description: "Preset zoom level",
toolbar: {
title: "Zoom Presets",
icon: "zoom",
items: [
{
// undefined so the there is no zoom value set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: extra word?

Suggested change
// undefined so the there is no zoom value set
// undefined so there is no zoom value set

value: undefined,
title: "default",
},
{
value: "2",
title: "200%",
},
{
value: "4",
title: "400%",
},
],
},
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 new controls in our toolbar! We can also configure these global parameters for specific stories

Screenshot 2025-03-20 at 1 33 43 PM Screenshot 2025-03-20 at 1 36 04 PM

},

tags: ["autodocs"],
Expand Down
149 changes: 109 additions & 40 deletions __docs__/components/all-variants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import {StyleSheet} from "aphrodite";
import {addStyle} from "@khanacademy/wonder-blocks-core";
import {
border,
breakpoint,
semanticColor,
sizing,
spacing,
} from "@khanacademy/wonder-blocks-tokens";

Expand All @@ -14,6 +16,7 @@ import {LabelLarge} from "@khanacademy/wonder-blocks-typography";
const StyledTable = addStyle("table");
const StyledTh = addStyle("th");
const StyledTd = addStyle("td");
const StyledUl = addStyle("ul");

type Variant = {name: string; props: StrictArgs};

Expand All @@ -22,7 +25,7 @@ type Props = {
* The children as a function that receives the state props used to render
* each variant of the component.
*/
children: (props: any) => React.ReactNode;
children: (props: any, name: string) => React.ReactNode;
/**
* The categories to display in the table as columns.
*/
Expand All @@ -31,59 +34,125 @@ type Props = {
* The states to display in the table as rows.
*/
columns: Array<Variant>;
/**
* The layout for AllVariants.
* - `responsive`: variants will be displayed in a table at larger screen
* sizes and in a list at smaller screen sizes
* - `list`: variants will always be displayed in a list
*/
layout?: "responsive" | "list";
};

/**
* A table that displays all possible variants of a component.
*/
export function AllVariants({children, columns, rows}: Props) {
export function AllVariants(props: Props) {
const {children, rows, columns, layout = "responsive"} = props;

return (
<StyledTable style={styles.table}>
<thead>
<tr>
<StyledTh style={styles.cell}>
<LabelLarge>Category / State</LabelLarge>
</StyledTh>
{columns.map((col, index) => (
<StyledTh key={index} scope="col" style={styles.cell}>
<LabelLarge>{col.name}</LabelLarge>
</StyledTh>
))}
</tr>
</thead>
<tbody>
{rows.map((row, idx) => (
<tr key={idx}>
<StyledTh scope="row" style={styles.cell}>
<LabelLarge>{row.name}</LabelLarge>
</StyledTh>
{columns.map((col) => (
<StyledTd
key={col.name}
style={[
styles.cell,
{
border: `${border.width.hairline}px dashed ${semanticColor.border.primary}`,
},
]}
>
{children({
...row.props,
...col.props,
})}
</StyledTd>
<>
{layout === "responsive" && (
<StyledTable style={[styles.table]}>
<thead>
<tr>
<StyledTh style={styles.cell}>
<LabelLarge>Category / State</LabelLarge>
</StyledTh>
{columns.map((col, index) => (
<StyledTh
key={index}
scope="col"
style={styles.cell}
>
<LabelLarge>{col.name}</LabelLarge>
</StyledTh>
))}
</tr>
</thead>
<tbody>
{rows.map((row, idx) => (
<tr key={idx}>
<StyledTh scope="row" style={styles.cell}>
<LabelLarge>{row.name}</LabelLarge>
</StyledTh>
{columns.map((col) => (
<StyledTd
key={col.name}
style={[
styles.cell,
{
border: `${border.width.hairline}px dashed ${semanticColor.border.primary}`,
},
]}
>
{children(
{
...row.props,
...col.props,
},
`${row.name} ${col.name}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting, I just did something super similar in #2509!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!! I was also addressing a11y warnings in the new stories! I see in #2509 we add aria-label to the props. I ended up adding a new argument for the name so it could be used for a unique aria-label. I was thinking not all components may support an aria-label at the root level so providing it as an arg keeps it flexible! What do you think?

)}
</StyledTd>
))}
</tr>
))}
</tr>
))}
</tbody>
</StyledTable>
</tbody>
</StyledTable>
)}
<StyledUl
style={[
styles.list,
layout === "responsive" && styles.listResponsive,
]}
>
{rows.map((row) => {
return columns.map((column) => {
return (
<li key={`${row.name} ${column.name}`}>
<LabelLarge>
Column: {column.name}, Row: {row.name}
</LabelLarge>

<div
style={{
padding: sizing.size_100,
marginBlock: sizing.size_100,
border: `${border.width.hairline}px dashed ${semanticColor.border.primary}`,
}}
>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: It would be great switching to View (or StyledDiv if view doesn't fit) to prevent inlining the CSS in the html.

Suggested change
<div
style={{
padding: sizing.size_100,
marginBlock: sizing.size_100,
border: `${border.width.hairline}px dashed ${semanticColor.border.primary}`,
}}
>
<View
style={{
padding: sizing.size_100,
marginBlock: sizing.size_100,
border: `${border.width.hairline}px dashed ${semanticColor.border.primary}`,
}}
>

Expanding on this, the styles could be moved to the styles object as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder! Updated :)

{children(
{...column.props, ...row.props},
`${row.name} ${column.name}`,
)}
</div>
</li>
);
});
})}
</StyledUl>
</>
);
}

const styles = StyleSheet.create({
table: {
borderCollapse: "collapse",
textAlign: "left",
[breakpoint.mediaQuery.smOrSmaller]: {
display: "none",
},
},
list: {
margin: 0,
padding: 0,
listStyle: "none",
display: "flex",
flexDirection: "column",
},
listResponsive: {
[breakpoint.mediaQuery.mdOrLarger]: {
display: "none",
},
},
cell: {
margin: spacing.medium_16,
Expand Down
43 changes: 43 additions & 0 deletions __docs__/components/scenarios-layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import React from "react";
import type {StrictArgs} from "@storybook/react";
import {StyleSheet} from "aphrodite";
import {View} from "@khanacademy/wonder-blocks-core";
import {LabelLarge} from "@khanacademy/wonder-blocks-typography";
import {sizing} from "@khanacademy/wonder-blocks-tokens";

type Props = {
scenarios: {name: string; props: StrictArgs}[];
children: (props: any, name: string) => React.ReactNode;
};

/**
* Useful for stories that show different scenarios.
*
* Normally, ScenariosLayout is used for different cases at rest state.
*/
export const ScenariosLayout = (props: Props) => {
const {scenarios, children} = props;
return (
<View style={styles.container}>
{scenarios.map((scenario) => {
return (
<View key={scenario.name} style={styles.scenario}>
<LabelLarge>{scenario.name}</LabelLarge>
{children(scenario.props, scenario.name)}
</View>
);
})}
</View>
);
};

const styles = StyleSheet.create({
container: {
gap: sizing.size_200,
alignItems: "flex-start",
},
scenario: {
gap: sizing.size_100,
maxWidth: "100%",
},
});
5 changes: 5 additions & 0 deletions __docs__/components/text-for-testing.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: This is super useful!

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const rtlText = "هذا الرابط مكتوب باللغة العربية";
export const longText =
"Lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua";
export const longTextWithNoWordBreak =
"Loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididuntutlaboreetdoloremagnaaliqua";
Loading