feat: adds app switcher lab component #2416

michaeltamaki-okta · 2024-11-15T21:33:49Z

Summary

feat: adds app switcher lab component

Building out the presentation layer of the "App Switcher" https://odyssey.okta.design/latest/components/app-switcher/design-mla5Wn5G (now called "Nav Rail").
Figma: https://www.figma.com/design/zh7A9gjaDlI50Hctdvd1OB/Odyssey-Components?node-id=22141-142254&node-type=frame&t=Qc7MjiWUFNtOh9Vd-0
Playground: https://6485e4e33982ca4977fff8d81ef98f680e713a9c.ods.dev/?path=/docs/labs-components-appswitcher--docs

Component checklist

Code structure

Organize code using a modular component architecture.
Use semantic HTML for clear and meaningful structure.
Break down complex components into smaller, reusable parts.

Accessibility implementation

Apply ARIA attributes to indicate roles and relationships.
Test focus management and ensure interactive elements are keyboard accessible.
Provide visual alternatives for auditory elements.

Internationalization (i18n) implementation

Separate any text content from the component's code for translation.
Test RTL support in Storybook

Cross-browser compatibility

Ensure consistent behavior and appearance across browsers.

Chrome

Safari

Firefox

Documentation

Include code snippets, demos, and examples for various use cases.
Provide guidance on customization and handling special cases.

Finalize code

Interaction designers, UI eng and visual designers need to review that the Storybook code matches the intended Figma design
Interaction designers need to confirm Storybook code matches Supernova
Interaction and visual designers merge Figma and code at the same time
Share updates to the #design-system-team

QA

Include necessary unit tests to continuously verify component functionality
Screen reader test error/success states or message
Test edge cases and error scenarios to validate robustness

Testing and validation

Share work in Craft to get feedback from the Odyssey team
Share work to get feedback from pillar design captains or product designers in #design-team as customer feedback is essential to help us validate our assumptions, identify pain points, and improve our user experience

packages/odyssey-react-mui/src/labs/NavRail/OktaAura.tsx

packages/odyssey-react-mui/src/labs/NavRail/NavRailApp.tsx

michaeltamaki-okta

Added notes from meeting with @KevinGhadyani-Okta

michaeltamaki-okta · 2024-11-18T19:10:19Z

packages/odyssey-react-mui/src/labs/NavRail/NavRailApp.tsx

+  const renderAppIconLink = useCallback(
+    (muiProps: MuiPropsContextType) => {
+      return (
+        <NavRailAppLinkComponent


Add aria-current

michaeltamaki-okta · 2024-11-18T19:12:05Z

packages/odyssey-react-mui/src/labs/NavRail/NavRail.tsx

+      >
+        <OktaAura />
+      </NavRailOktaAuraWrapperComponent>
+      {appIcons.map((appIcon) => (


Add nav (Add to the NavRail wrapper), ul (Add a wrapper over all the apps), li (Add to NavRailApp component)

michaeltamaki-okta · 2024-11-18T19:21:17Z

packages/odyssey-react-mui/src/labs/NavRail/NavRail.tsx

+  shouldForwardProp: (prop) => prop !== "odysseyDesignTokens",
+})(({ odysseyDesignTokens }: { odysseyDesignTokens: DesignTokens }) => ({
+  width: "100%",
+  height: NAV_RAIL_WIDTH,


See if we can import a shared value between top nav

michaeltamaki-okta · 2024-11-18T19:25:54Z

packages/odyssey-react-mui/src/labs/NavRail/NavRailApp.tsx

+const APP_SIDE_LENGTH = "36px";
+const APP_ICON_SIDE_LENGTH = "32px";


Change to rem

michaeltamaki-okta · 2024-11-18T19:28:58Z

packages/odyssey-react-mui/src/labs/NavRail/NavRail.tsx

+const NAV_RAIL_WIDTH = "64px";
+
+export type NavRailProps = {
+  appIcons?: Omit<NavRailAppProps, "selectedAppName">[];


Prefer using Pick instead of Omit

michaeltamaki-okta · 2024-11-18T19:29:41Z

packages/odyssey-react-mui/src/labs/NavRail/NavRailApp.tsx

+    (muiProps: MuiPropsContextType) => {
+      return (


implicit return

michaeltamaki-okta · 2024-11-18T19:31:32Z

packages/odyssey-react-mui/src/labs/NavRail/NavRailApp.tsx

+        >
+          <NavRailAppImgComponent
+            src={isSelected ? appIconSelectedUrl : appIconDefaultUrl}
+            alt="" // Tell screen reader to ignore the image; the Tooltip describes the link


role=presentation

michaeltamaki-okta · 2024-11-18T19:48:19Z

packages/odyssey-react-mui/src/labs/NavRail/NavRail.tsx

+
+const NAV_RAIL_WIDTH = "64px";
+
+export type NavRailProps = {


Add visibilityState = invisible | loading | loaded

How should we handle the loading state?

Skeleton

Enduser: Skeleton shows, then disappears (pop in during load)

Admin: Skeleton shows, then gets populated

No Skeleton ✅

Enduser: Skeleton does not show, then disappears

Admin: Skeleton does not show, then nav rail pops in

Could we mitigate this via storing in local storage to prevent pop in? Would pop in the first load, but would not pop in after that.

Also consider that this is just for the MVP where it is only shown for the admin. In the future, when it is shown to all users, we could switch to skeleton.

Full screen loading screen (prevents pop in)

Would block showing the app until we get the data. Unacceptable to downstream teams.

Keep side nav the same width regardless of app switcher

No, would break design assumptions

michaeltamaki-okta · 2024-11-18T19:56:17Z

packages/odyssey-storybook/src/components/odyssey-labs/NavRail/NavRail.stories.tsx

+  play: async ({ canvasElement, step }: PlaywrightProps<NavRailProps>) => {
+    const canvas = within(canvasElement);
+
+    await step("Nav rail successfully loads", async () => {


shows all items

michaeltamaki-okta · 2024-11-18T19:56:33Z

packages/odyssey-storybook/src/components/odyssey-labs/NavRail/NavRail.stories.tsx

+  play: async ({ canvasElement, step }: PlaywrightProps<NavRailProps>) => {
+    const canvas = within(canvasElement);
+
+    await step("Nav rail does not load", async () => {


hides when no items

bryancunningham-okta · 2024-11-20T17:28:59Z

packages/odyssey-react-mui/src/labs/AppSwitcher/AppSwitcher.tsx

+} from "./AppSwitcherApp";
+import { TOP_NAV_HEIGHT } from "../TopNav";
+
+const APP_SWITCHER_WIDTH = `${64 / 14}rem`;


@michaeltamaki-okta Is this value meant to scale with the font size? Or, should it be a static px value?

If it should be a rem value, we should probably use one of our spacing tokens here

That would be Spacing9 I think. The same as top nav.

bryancunningham-okta · 2024-11-20T17:29:48Z

packages/odyssey-react-mui/src/labs/AppSwitcher/AppSwitcher.tsx

+  display: "flex",
+  alignItems: "center",
+  justifyContent: "center",
+  marginBottom: odysseyDesignTokens.Spacing4,


Suggested change

marginBottom: odysseyDesignTokens.Spacing4,

marginBlockEnd: odysseyDesignTokens.Spacing4,

bryancunningham-okta · 2024-11-20T17:33:35Z

packages/odyssey-react-mui/src/labs/AppSwitcher/AppSwitcherApp.tsx

+
+const APP_SIDE_LENGTH_VAL = 36;
+const APP_SIDE_LENGTH_REM = `${APP_SIDE_LENGTH_VAL / 14}rem`;
+const APP_ICON_SIDE_LENGTH = `${32 / 14}rem`;


We should probably be using token values for these. If we don't have token values for these sizes already, the sizes are likely incorrect. We'd have to go back to design and tell them we're using a token that already exists

bryancunningham-okta · 2024-11-20T17:34:35Z

packages/odyssey-react-mui/src/labs/AppSwitcher/AppSwitcherApp.tsx

+    alignItems: "center",
+    justifyContent: "center",
+    margin: 0,
+    width: APP_SIDE_LENGTH_REM,


The img size + the padding should be setting the width/height on these. We shouldn't need to set an explicit size here, I wouldn't think

benschell-okta

I do not feel qualified to review the core component, but the changes UiShell looks exactly like I hoped/expected (and that's primarily what my approval relates to)! Great work!

KevinGhadyani-Okta · 2024-11-20T18:39:46Z

packages/odyssey-react-mui/src/labs/AppSwitcher/AppSwitcher.tsx

+      </AppSwitcherOktaAuraWrapperComponent>
+      <AppSwitcherAppIconULComponent>
+        {isLoading
+          ? [...Array(3)].map(() => <AppSwitcherAppSkeleton />)


I'd prefer this syntax:

Suggested change

? [...Array(3)].map(() => <AppSwitcherAppSkeleton />)

? Array(3).fill().map(() => <AppSwitcherAppSkeleton />)

I don't like using [...something] when we could use a semantic method like .slice() or .concat instead. Your intent is more clear that way.

KevinGhadyani-Okta · 2024-11-20T18:45:54Z

packages/odyssey-react-mui/src/labs/AppSwitcher/AppSwitcherApp.tsx

+  selectedAppName,
+}: AppSwitcherAppProps) => {
+  const odysseyDesignTokens = useOdysseyDesignTokens();
+  const isSelected = appName === selectedAppName;


You got selectedAppName in the API?

We'd have to include this as part of renderUiShell, it is not returned in the API response

KevinGhadyani-Okta · 2024-11-20T18:48:27Z

packages/odyssey-storybook/src/components/odyssey-labs/AppSwitcher/AppSwitcher.stories.tsx

+      await expect(canvas.getAllByRole("navigation")).toHaveLength(1);
+      await expect(canvas.queryAllByRole("listitem")).toHaveLength(0);
+      await expect(canvas.queryAllByRole("link")).toHaveLength(0);
+    });


You can check on the navigation .toBeVisible() which is more accurate to your intent. You don't care if it's in the DOM.

I wonder if navigations should have an aria label because someone's not gonna know what they're selecting. I think there's a difference between primary and secondary navigations in ARIA. Something you'll wanna check into.

If so, then you'd do getByRole("navigation", { name: "YOUR_LABEL" }).

KevinGhadyani-Okta

Looks good! I made a few small comments.

michaeltamaki-okta requested a review from a team as a code owner November 15, 2024 21:33

michaeltamaki-okta mentioned this pull request Nov 15, 2024

feat: adds nav rail lab component #2415

Closed

20 tasks

michaeltamaki-okta commented Nov 15, 2024

View reviewed changes

packages/odyssey-react-mui/src/labs/NavRail/OktaAura.tsx Outdated Show resolved Hide resolved

packages/odyssey-react-mui/src/labs/NavRail/NavRailApp.tsx Outdated Show resolved Hide resolved

michaeltamaki-okta commented Nov 18, 2024

View reviewed changes

michaeltamaki-okta changed the title ~~feat: adds nav rail lab component~~ feat: adds app switcher lab component Nov 18, 2024

michaeltamaki-okta force-pushed the mt-nav-rail branch 3 times, most recently from 6485e4e to 503f6d7 Compare November 20, 2024 01:26

bryancunningham-okta reviewed Nov 20, 2024

View reviewed changes

benschell-okta previously approved these changes Nov 20, 2024

View reviewed changes

KevinGhadyani-Okta reviewed Nov 20, 2024

View reviewed changes

michaeltamaki-okta dismissed benschell-okta’s stale review via 83fd141 November 20, 2024 20:16

bryancunningham-okta previously approved these changes Nov 20, 2024

View reviewed changes

michaeltamaki-okta dismissed bryancunningham-okta’s stale review via 3f532de November 20, 2024 21:22

KevinGhadyani-Okta approved these changes Nov 20, 2024

View reviewed changes

michaeltamaki-okta added 4 commits November 20, 2024 12:15

feat: adds nav rail lab component

2aac1db

fix: initialVisibleSections

85b29c6

fix: remove hard coded img sizes

ea44bd0

fix: less svg padding

305c19d

michaeltamaki-okta force-pushed the mt-nav-rail branch from 3f532de to 305c19d Compare November 20, 2024 22:15

oktapp-aperture-okta bot merged commit 1b48073 into main Nov 20, 2024
1 check passed

oktapp-aperture-okta bot deleted the mt-nav-rail branch November 20, 2024 22:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds app switcher lab component #2416

feat: adds app switcher lab component #2416

michaeltamaki-okta commented Nov 15, 2024 •

edited

Loading

michaeltamaki-okta left a comment

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

michaeltamaki-okta Nov 18, 2024

bryancunningham-okta Nov 20, 2024

bryancunningham-okta Nov 20, 2024

KevinGhadyani-Okta Nov 20, 2024

bryancunningham-okta Nov 20, 2024

bryancunningham-okta Nov 20, 2024

bryancunningham-okta Nov 20, 2024 •

edited

Loading

benschell-okta left a comment

KevinGhadyani-Okta Nov 20, 2024 •

edited

Loading

KevinGhadyani-Okta Nov 20, 2024

michaeltamaki-okta Nov 20, 2024 •

edited

Loading

KevinGhadyani-Okta Nov 20, 2024 •

edited

Loading

KevinGhadyani-Okta left a comment

		const APP_SIDE_LENGTH = "36px";
		const APP_ICON_SIDE_LENGTH = "32px";

	marginBottom: odysseyDesignTokens.Spacing4,
	marginBlockEnd: odysseyDesignTokens.Spacing4,

	? [...Array(3)].map(() => <AppSwitcherAppSkeleton />)
	? Array(3).fill().map(() => <AppSwitcherAppSkeleton />)

feat: adds app switcher lab component #2416

feat: adds app switcher lab component #2416

Conversation

michaeltamaki-okta commented Nov 15, 2024 • edited Loading

Summary

Component checklist

Code structure

Accessibility implementation

Internationalization (i18n) implementation

Cross-browser compatibility

Chrome

Safari

Firefox

Documentation

Finalize code

QA

Testing and validation

michaeltamaki-okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

How should we handle the loading state?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryancunningham-okta Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

benschell-okta left a comment

Choose a reason for hiding this comment

KevinGhadyani-Okta Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaeltamaki-okta Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

KevinGhadyani-Okta Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

KevinGhadyani-Okta left a comment

Choose a reason for hiding this comment

michaeltamaki-okta commented Nov 15, 2024 •

edited

Loading

bryancunningham-okta Nov 20, 2024 •

edited

Loading

KevinGhadyani-Okta Nov 20, 2024 •

edited

Loading

michaeltamaki-okta Nov 20, 2024 •

edited

Loading

KevinGhadyani-Okta Nov 20, 2024 •

edited

Loading