From f0e467830718a10a14c671511fc2aa697373ccaf Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 30 Jan 2025 10:21:32 +0530 Subject: [PATCH 1/7] Improve accessibility and keyboard access --- .../components/Sidebar/SubscriptionCard.tsx | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx b/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx index 513a0ac994..d72c0972b9 100644 --- a/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx +++ b/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx @@ -1,5 +1,6 @@ import { Overlay } from "@/base/components/containers"; import type { ButtonishProps } from "@/base/components/mui"; +import { UnstyledButton } from "@/new/photos/components/UnstyledButton"; import type { UserDetails } from "@/new/photos/services/user-details"; import { familyUsage, @@ -56,7 +57,11 @@ export const SubscriptionCard: React.FC = ({ const BackgroundOverlay: React.FC = () => ( ( ); const ClickOverlay: React.FC = ({ onClick }) => ( - + - + ); +/** A mixin of UnstyledButton and {@link Overlay}, plus custom styling. */ +const ClickOverlayButton = styled(UnstyledButton)` + /* Overlay */ + position: absolute; + width: 100%; + height: 100%; + top: 0; + left: 0; + + /* Position the chevron at the middle right */ + display: flex; + justify-content: flex-end; + align-items: center; + + /* Reset the button color */ + color: inherit; +`; + interface SubscriptionCardContentOverlayProps { userDetails: UserDetails; } From a7a21e66a7b3c6149d4da68f281d6eca03d9f294 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 30 Jan 2025 10:44:09 +0530 Subject: [PATCH 2/7] Style the focus and activation --- .../components/Sidebar/SubscriptionCard.tsx | 9 ++++++--- .../new/photos/components/UnstyledButton.tsx | 19 +++++++++++++++++++ .../new/photos/components/gallery/BarImpl.tsx | 6 +++--- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx b/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx index d72c0972b9..368e65d736 100644 --- a/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx +++ b/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx @@ -1,6 +1,6 @@ import { Overlay } from "@/base/components/containers"; import type { ButtonishProps } from "@/base/components/mui"; -import { UnstyledButton } from "@/new/photos/components/UnstyledButton"; +import { FocusVisibleUnstyledButton } from "@/new/photos/components/UnstyledButton"; import type { UserDetails } from "@/new/photos/services/user-details"; import { familyUsage, @@ -74,8 +74,11 @@ const ClickOverlay: React.FC = ({ onClick }) => ( ); -/** A mixin of UnstyledButton and {@link Overlay}, plus custom styling. */ -const ClickOverlayButton = styled(UnstyledButton)` +/** + * The transparent button element offers activation of the subscription card is activated on clicks + * A mixin of UnstyledButton, FocusVisibleUnstyledButton, {@link Overlay}, plus + * custom styling. */ +const ClickOverlayButton = styled(FocusVisibleUnstyledButton)` /* Overlay */ position: absolute; width: 100%; diff --git a/web/packages/new/photos/components/UnstyledButton.tsx b/web/packages/new/photos/components/UnstyledButton.tsx index 920c646065..d3d5d6f590 100644 --- a/web/packages/new/photos/components/UnstyledButton.tsx +++ b/web/packages/new/photos/components/UnstyledButton.tsx @@ -24,3 +24,22 @@ export const UnstyledButton = styled("button")` /* Default cursor on mouse over of a button is not a hand pointer */ cursor: pointer; `; + +/** + * A variant of {@link UnstyledButton} that modifying the keyboard focus and + * activation states to match the styling of {@link FocusVisibleButton}. + */ +export const FocusVisibleUnstyledButton = styled(UnstyledButton)( + ({ theme }) => ` + &:focus-visible { + outline: 1px solid ${theme.vars.palette.stroke.base}; + outline-offset: 2px; + border-radius: 2px; + } + &:active { + outline: 1px solid ${theme.vars.palette.stroke.faint}; + outline-offset: 1px; + border-radius: 2px; + } +`, +); diff --git a/web/packages/new/photos/components/gallery/BarImpl.tsx b/web/packages/new/photos/components/gallery/BarImpl.tsx index c648c11ae0..9dada6a492 100644 --- a/web/packages/new/photos/components/gallery/BarImpl.tsx +++ b/web/packages/new/photos/components/gallery/BarImpl.tsx @@ -12,7 +12,7 @@ import { ItemCard, TileTextOverlay, } from "@/new/photos/components/Tiles"; -import { UnstyledButton } from "@/new/photos/components/UnstyledButton"; +import { FocusVisibleUnstyledButton } from "@/new/photos/components/UnstyledButton"; import type { CollectionSummary, CollectionSummaryType, @@ -42,8 +42,8 @@ import { type ListChildComponentProps, areEqual, } from "react-window"; -import { isMLSupported } from "../../services/ml"; import type { GalleryBarMode } from "./reducer"; +import { isMLSupported } from "../../services/ml"; export interface GalleryBarImplProps { /** @@ -352,7 +352,7 @@ const ModeIndicator: React.FC< ); }; -const ModeButton = styled(UnstyledButton, { +const ModeButton = styled(FocusVisibleUnstyledButton, { shouldForwardProp: (propName) => propName != "active", })<{ active: boolean }>( ({ theme, active }) => ` From 13860afbc237ce3ff85df287c03df3c6f85917ff Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 30 Jan 2025 10:50:39 +0530 Subject: [PATCH 3/7] Tweak --- .../components/Sidebar/SubscriptionCard.tsx | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx b/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx index 368e65d736..029e48c1a2 100644 --- a/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx +++ b/web/apps/photos/src/components/Sidebar/SubscriptionCard.tsx @@ -1,6 +1,6 @@ import { Overlay } from "@/base/components/containers"; import type { ButtonishProps } from "@/base/components/mui"; -import { FocusVisibleUnstyledButton } from "@/new/photos/components/UnstyledButton"; +import { UnstyledButton } from "@/new/photos/components/UnstyledButton"; import type { UserDetails } from "@/new/photos/services/user-details"; import { familyUsage, @@ -75,10 +75,13 @@ const ClickOverlay: React.FC = ({ onClick }) => ( ); /** - * The transparent button element offers activation of the subscription card is activated on clicks - * A mixin of UnstyledButton, FocusVisibleUnstyledButton, {@link Overlay}, plus - * custom styling. */ -const ClickOverlayButton = styled(FocusVisibleUnstyledButton)` + * The transparent button element offers activation of the subscription card. + * + * A mixin of {@link FocusVisibleUnstyledButton} and {@link Overlay}, plus + * custom styling to place its contents (chevron) at the middle right. + */ +const ClickOverlayButton = styled(UnstyledButton)( + ({ theme }) => ` /* Overlay */ position: absolute; width: 100%; @@ -93,7 +96,21 @@ const ClickOverlayButton = styled(FocusVisibleUnstyledButton)` /* Reset the button color */ color: inherit; -`; + + /* FocusVisibleUnstyledButton, but customized to work button with the larger + subscription card and its border radii. */ + &:focus-visible { + outline: 1.5px solid ${theme.vars.palette.stroke.base}; + outline-offset: 2px; + border-radius: 3px; + } + &:active { + outline: 2px solid ${theme.vars.palette.stroke.faint}; + outline-offset: 1px; + border-radius: 3px; + } +`, +); interface SubscriptionCardContentOverlayProps { userDetails: UserDetails; From 2bd36b079587bfc4cd39f7099a4ce94a426de95b Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 30 Jan 2025 11:17:04 +0530 Subject: [PATCH 4/7] Improve vis --- web/packages/base/components/utils/theme.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/web/packages/base/components/utils/theme.ts b/web/packages/base/components/utils/theme.ts index 2be993855f..109a4a87ba 100644 --- a/web/packages/base/components/utils/theme.ts +++ b/web/packages/base/components/utils/theme.ts @@ -184,8 +184,7 @@ const _colors = { baseHover: "rgba(0 0 0 / 0.87)", muted: "rgba(0 0 0 / 0.12)", faint: "rgba(0 0 0 / 0.04)", - // TODO(LM): Different from Figma. - faintHover: "rgba(0 0 0 / 0.06)", + faintHover: "rgba(0 0 0 / 0.08)", fainter: "rgba(0 0 0 / 0.02)", }, stroke: { @@ -295,8 +294,9 @@ const getColorSchemes = (colors: ReturnType) => ({ // The color of an hovered action. hover: colors.light.fill.faintHover, // For an icon button, the hover background color is derived - // from the active color above and this opacity. - hoverOpacity: 0.06, + // from the active color above and this opacity. Use a value + // that results in the same result as faintHover. + hoverOpacity: 0.08, // TODO(LM): Remove commented. // The color of a selected action. // @@ -323,7 +323,7 @@ const getColorSchemes = (colors: ReturnType) => ({ bg: colors.light.fill.faint, hoverBg: colors.light.fill.faintHover, // We don't use this currently. - // disabledBg: "#ff0000", //colors.light.fill.faint, + // disabledBg: colors.light.fill.fainter, }, }, }, From aea5f78765737be33ff905be647b53b9dc443f05 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 30 Jan 2025 11:22:51 +0530 Subject: [PATCH 5/7] Lift it up for dark mode --- web/packages/base/components/utils/theme.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/packages/base/components/utils/theme.ts b/web/packages/base/components/utils/theme.ts index 109a4a87ba..8810e67da3 100644 --- a/web/packages/base/components/utils/theme.ts +++ b/web/packages/base/components/utils/theme.ts @@ -220,7 +220,7 @@ const _colors = { baseHover: "rgba(255 255 255 / 0.90)", muted: "rgba(255 255 255 / 0.16)", faint: "rgba(255 255 255 / 0.12)", - faintHover: "rgba(255 255 255 / 0.06)", + faintHover: "rgba(255 255 255 / 0.16)", fainter: "rgba(255 255 255 / 0.05)", }, stroke: { @@ -385,7 +385,7 @@ const getColorSchemes = (colors: ReturnType) => ({ hover: colors.dark.fill.faintHover, // For an icon button, the hover background color is derived // from the active color above and this opacity. - hoverOpacity: 0.12, + hoverOpacity: 0.16, // The color of a selected action. // // Placeholder; not clear how it impacts us. From 08c4842f20e1f7f160e2deb7fa682c14d256ba7f Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 30 Jan 2025 11:25:38 +0530 Subject: [PATCH 6/7] Stop duplicating comments --- web/packages/base/components/utils/theme.ts | 29 ++------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/web/packages/base/components/utils/theme.ts b/web/packages/base/components/utils/theme.ts index 8810e67da3..42f4b81385 100644 --- a/web/packages/base/components/utils/theme.ts +++ b/web/packages/base/components/utils/theme.ts @@ -327,6 +327,7 @@ const getColorSchemes = (colors: ReturnType) => ({ }, }, }, + // -- See the light mode section for comments dark: { palette: { background: colors.dark.background, @@ -356,15 +357,9 @@ const getColorSchemes = (colors: ReturnType) => ({ contrastText: colors.fixed.white, }, text: { - // Alias the tokens used by MUI to the ones that we use. This way, - // we don't need to change the default ("primary"), or update the - // MUI internal styling that refers to these tokens. - // - // Our own code should not use these. primary: colors.dark.text.base, secondary: colors.dark.text.muted, disabled: colors.dark.text.faint, - // Our color tokens. base: colors.dark.text.base, muted: colors.dark.text.muted, faint: colors.dark.text.faint, @@ -374,43 +369,23 @@ const getColorSchemes = (colors: ReturnType) => ({ divider: colors.dark.stroke.faint, fixed: colors.fixed, boxShadow: colors.dark.boxShadow, - // Override some MUI defaults for styling action elements like - // buttons and menu items. - // - // https://github.com/mui/material-ui/blob/v6.4.0/packages/mui-material/src/styles/createPalette.js#L68 + // -- See the light mode section for comments action: { - // The color of an active action, like an icon button. active: colors.dark.stroke.base, - // The color of an hovered action. hover: colors.dark.fill.faintHover, - // For an icon button, the hover background color is derived - // from the active color above and this opacity. hoverOpacity: 0.16, - // The color of a selected action. - // - // Placeholder; not clear how it impacts us. // selected: colors.dark.stroke.base, // selectedOpacity: 0.08, - // The color of a disabled action (including regular buttons). disabled: colors.dark.text.faint, // disabledOpacity: 0.12, - // The background color of a disabled action. disabledBackground: colors.dark.fill.faint, - // Placeholder; not clear how it impacts us. // focus: colors.dark.stroke.base, - // Placeholder (MUI default); not clear how it impacts us. // focusOpacity: 1, - // Placeholder (MUI default); not clear how it impacts us. // activatedOpacity: 0.12, }, - // Override some internal MUI defaults that impact the components - // which we use. - // - // https://github.com/mui/material-ui/blob/v6.4.0/packages/mui-material/src/styles/createThemeWithVars.js#L271 FilledInput: { bg: colors.dark.fill.faint, hoverBg: colors.dark.fill.faintHover, - // We don't use this currently. // disabledBg: colors.dark.fill.faint, }, }, From 09bfca7aa427880d5f5002168320b2359c7579f9 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 30 Jan 2025 12:38:44 +0530 Subject: [PATCH 7/7] lf --- web/packages/new/photos/components/gallery/BarImpl.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/new/photos/components/gallery/BarImpl.tsx b/web/packages/new/photos/components/gallery/BarImpl.tsx index 9dada6a492..c8330a42d1 100644 --- a/web/packages/new/photos/components/gallery/BarImpl.tsx +++ b/web/packages/new/photos/components/gallery/BarImpl.tsx @@ -42,8 +42,8 @@ import { type ListChildComponentProps, areEqual, } from "react-window"; -import type { GalleryBarMode } from "./reducer"; import { isMLSupported } from "../../services/ml"; +import type { GalleryBarMode } from "./reducer"; export interface GalleryBarImplProps { /**