From 994813dd99765533b2ed14c8386d052ed97783d5 Mon Sep 17 00:00:00 2001 From: Oleg Isonen Date: Mon, 14 Oct 2024 16:44:00 +0100 Subject: [PATCH] feat: Switch to 1px outline for all inputs and buttons, as well as use background color in lists (#4276) ## Description Most controls are affected 2px > 1px Navigator and project settings lists now use background color instead of outline ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file --- apps/builder/app/auth/brand-button.tsx | 2 +- .../features/project-settings/project-settings.tsx | 8 +++----- .../builder/features/sidebar-left/sidebar-tabs.tsx | 13 ++----------- .../sections/layout/shared/flex-grid.tsx | 10 +++------- .../style-panel/sections/position/inset-control.tsx | 3 ++- .../features/style-panel/sections/space/layout.tsx | 2 +- .../features/style-panel/shared/color-thumb.tsx | 4 ++++ .../style-panel/style-source/style-source-input.tsx | 9 ++------- .../builder/app/builder/shared/code-editor-base.tsx | 1 - .../shared/image-manager/image-thumbnail.tsx | 2 +- apps/builder/app/dashboard/shared/card.tsx | 4 ++-- .../src/components/__DEPRECATED__/list.tsx | 2 +- packages/design-system/src/components/button.tsx | 2 +- packages/design-system/src/components/checkbox.tsx | 2 +- .../design-system/src/components/component-card.tsx | 4 ++-- .../src/components/css-value-list-item.tsx | 11 ----------- packages/design-system/src/components/focus-ring.ts | 12 ++++-------- .../design-system/src/components/icon-button.tsx | 7 +++++-- .../design-system/src/components/input-field.tsx | 7 +++---- .../design-system/src/components/panel-tabs.tsx | 4 ++-- .../design-system/src/components/position-grid.tsx | 11 ++++++----- .../src/components/primitives/small-button.tsx | 2 +- .../design-system/src/components/select-button.tsx | 1 - packages/design-system/src/components/switch.tsx | 2 +- packages/design-system/src/components/text-area.tsx | 1 - packages/design-system/src/components/toolbar.tsx | 5 +---- packages/design-system/src/components/tree.tsx | 4 +--- 27 files changed, 50 insertions(+), 85 deletions(-) diff --git a/apps/builder/app/auth/brand-button.tsx b/apps/builder/app/auth/brand-button.tsx index e801c007109..d9049a76c93 100644 --- a/apps/builder/app/auth/brand-button.tsx +++ b/apps/builder/app/auth/brand-button.tsx @@ -26,7 +26,7 @@ export const buttonStyle = css({ boxShadow: theme.shadows.brandElevationBig, }, "&:focus-visible": { - outline: `2px solid ${theme.colors.borderFocus}`, + outline: `1px solid ${theme.colors.borderFocus}`, outlineOffset: 1, }, "&:disabled": { diff --git a/apps/builder/app/builder/features/project-settings/project-settings.tsx b/apps/builder/app/builder/features/project-settings/project-settings.tsx index 04f51c1ca54..7fcefa841a4 100644 --- a/apps/builder/app/builder/features/project-settings/project-settings.tsx +++ b/apps/builder/app/builder/features/project-settings/project-settings.tsx @@ -9,7 +9,6 @@ import { Flex, List, ListItem, - focusRingStyle, Text, } from "@webstudio-is/design-system"; import { $isProjectSettingsOpen } from "~/shared/nano-states/seo"; @@ -19,8 +18,6 @@ import { useState } from "react"; import { SectionMarketplace } from "./section-marketplace"; import { leftPanelWidth, rightPanelWidth } from "./utils"; -const focusOutline = focusRingStyle(); - const sectionNames = ["General", "Redirects", "Marketplace"]; type SectionName = (typeof sectionNames)[number]; @@ -72,8 +69,9 @@ export const ProjectSettingsView = ({ height: theme.spacing[13], px: theme.spacing[9], outline: "none", - "&:focus-visible": focusOutline, - "&:hover": focusOutline, + "&:focus-visible, &:hover": { + background: theme.colors.backgroundHover, + }, "&[aria-current=true]": { background: theme.colors.backgroundItemCurrent, color: theme.colors.foregroundMain, diff --git a/apps/builder/app/builder/features/sidebar-left/sidebar-tabs.tsx b/apps/builder/app/builder/features/sidebar-left/sidebar-tabs.tsx index 39071c9ecbb..40e78694b58 100644 --- a/apps/builder/app/builder/features/sidebar-left/sidebar-tabs.tsx +++ b/apps/builder/app/builder/features/sidebar-left/sidebar-tabs.tsx @@ -6,6 +6,7 @@ import { TabsTrigger, Tooltip, css, + focusRingStyle, styled, theme, } from "@webstudio-is/design-system"; @@ -19,17 +20,7 @@ export const SidebarTabs = styled(Tabs, { boxSizing: "border-box", }); -const triggerFocusRing = { - "&::after": { - content: '""', - position: "absolute", - inset: 4, - outlineWidth: 2, - outlineStyle: "solid", - outlineColor: theme.colors.borderFocus, - borderRadius: theme.borderRadius[3], - }, -}; +const triggerFocusRing = focusRingStyle(); const buttonStyle = css({ position: "relative", diff --git a/apps/builder/app/builder/features/style-panel/sections/layout/shared/flex-grid.tsx b/apps/builder/app/builder/features/style-panel/sections/layout/shared/flex-grid.tsx index 395f95537f8..f2720fc5026 100644 --- a/apps/builder/app/builder/features/style-panel/sections/layout/shared/flex-grid.tsx +++ b/apps/builder/app/builder/features/style-panel/sections/layout/shared/flex-grid.tsx @@ -76,20 +76,16 @@ export const FlexGrid = () => { height: 72, padding: theme.spacing[4], borderRadius: theme.borderRadius[4], + outline: "none", + border: `1px solid ${color}`, background: theme.colors.backgroundControls, alignItems: "center", gap: 0, gridTemplateColumns: "repeat(3, 1fr)", gridTemplateRows: "repeat(3, 1fr)", color, - outlineStyle: "solid", - outlineWidth: styleValueSourceColor === "default" ? 1 : 2, - outlineOffset: styleValueSourceColor === "default" ? -1 : -2, - outlineColor: color, "&:focus-within": { - outlineWidth: 2, - outlineOffset: -2, - outlineColor: theme.colors.borderLocalFlexUi, + borderColor: theme.colors.borderLocalFlexUi, }, }} > diff --git a/apps/builder/app/builder/features/style-panel/sections/position/inset-control.tsx b/apps/builder/app/builder/features/style-panel/sections/position/inset-control.tsx index 9f00aaa7391..722f9c6efd0 100644 --- a/apps/builder/app/builder/features/style-panel/sections/position/inset-control.tsx +++ b/apps/builder/app/builder/features/style-panel/sections/position/inset-control.tsx @@ -127,7 +127,8 @@ export const InsetControl = () => { height: theme.spacing[18], "&:focus-visible": { borderRadius: theme.borderRadius[3], - outline: `2px solid ${theme.colors.blue10}`, + outline: `1px solid ${theme.colors.blue10}`, + outlineOffset: -1, }, }} onFocus={keyboardNavigation.handleFocus} diff --git a/apps/builder/app/builder/features/style-panel/sections/space/layout.tsx b/apps/builder/app/builder/features/style-panel/sections/space/layout.tsx index 995f5ec0176..3fe6ffb2969 100644 --- a/apps/builder/app/builder/features/style-panel/sections/space/layout.tsx +++ b/apps/builder/app/builder/features/style-panel/sections/space/layout.tsx @@ -150,7 +150,7 @@ const Container = styled("div", { // (both in z-order and in top/left) [`&:focus-visible > ${Grid}`]: { borderRadius: theme.borderRadius[3], - outline: `2px solid ${theme.colors.blue10}`, + outline: `1px solid ${theme.colors.borderFocus}`, }, }); diff --git a/apps/builder/app/builder/features/style-panel/shared/color-thumb.tsx b/apps/builder/app/builder/features/style-panel/shared/color-thumb.tsx index eaa5086f73e..711efac02f8 100644 --- a/apps/builder/app/builder/features/style-panel/shared/color-thumb.tsx +++ b/apps/builder/app/builder/features/style-panel/shared/color-thumb.tsx @@ -59,6 +59,10 @@ const thumbStyle = css({ borderRadius: theme.borderRadius[2], borderWidth: 0, borderStyle: "solid", + "&:focus-visible": { + outline: `1px solid ${theme.colors.borderFocus}`, + outlineOffset: 1, + }, }); type Props = Omit, "color"> & { diff --git a/apps/builder/app/builder/features/style-panel/style-source/style-source-input.tsx b/apps/builder/app/builder/features/style-panel/style-source/style-source-input.tsx index 556ad3b3790..e89bb7db784 100644 --- a/apps/builder/app/builder/features/style-panel/style-source/style-source-input.tsx +++ b/apps/builder/app/builder/features/style-panel/style-source/style-source-input.tsx @@ -86,8 +86,7 @@ const TextFieldContainer = styled("div", { minWidth: 0, border: `1px solid ${theme.colors.borderMain}`, "&:focus-within": { - outline: `2px solid ${theme.colors.borderFocus}`, - outlineOffset: -1, + borderColor: theme.colors.borderFocus, }, }); @@ -173,16 +172,12 @@ const TextFieldBase: ForwardRefRenderFunction< {...textFieldProps} variant="chromeless" css={{ - color: theme.colors.hiContrast, fontVariantNumeric: "tabular-nums", - fontFamily: theme.fonts.sans, - fontSize: theme.deprecatedFontSize[3], lineHeight: 1, order: 1, - border: "none", flex: 1, "&:focus-within, &:hover": { - outline: "none", + borderColor: "transparent", }, }} size="1" diff --git a/apps/builder/app/builder/shared/code-editor-base.tsx b/apps/builder/app/builder/shared/code-editor-base.tsx index c096f9a999b..8eaab8f5094 100644 --- a/apps/builder/app/builder/shared/code-editor-base.tsx +++ b/apps/builder/app/builder/shared/code-editor-base.tsx @@ -85,7 +85,6 @@ const editorContentStyle = css({ userSelect: "text", "&:focus-within": { borderColor: theme.colors.borderFocus, - outline: `1px solid ${theme.colors.borderFocus}`, }, '&[data-invalid="true"]': { borderColor: theme.colors.borderDestructiveMain, diff --git a/apps/builder/app/builder/shared/image-manager/image-thumbnail.tsx b/apps/builder/app/builder/shared/image-manager/image-thumbnail.tsx index b1b6afc290c..4d10e080f26 100644 --- a/apps/builder/app/builder/shared/image-manager/image-thumbnail.tsx +++ b/apps/builder/app/builder/shared/image-manager/image-thumbnail.tsx @@ -56,7 +56,7 @@ const ThumbnailContainer = styled(Box, { }, state: { selected: { - boxShadow: `0px 0px 0px 2px ${theme.colors.blue10}, 0px 0px 0px 2px ${theme.colors.blue10}`, + outline: `1px solid ${theme.colors.borderFocus}`, }, }, }, diff --git a/apps/builder/app/dashboard/shared/card.tsx b/apps/builder/app/dashboard/shared/card.tsx index 20c938c1184..d9f31da4684 100644 --- a/apps/builder/app/dashboard/shared/card.tsx +++ b/apps/builder/app/dashboard/shared/card.tsx @@ -24,12 +24,12 @@ const cardStyle = css({ borderColor: theme.colors.borderMain, borderRadius: theme.borderRadius[4], background: theme.colors.brandBackgroundProjectCardFront, + outline: "none", "&:hover, &:focus-within": { boxShadow: theme.shadows.brandElevationBig, }, "&:focus-visible": { - outline: `2px solid ${theme.colors.borderFocus}`, - outlineOffset: 1, + borderColor: theme.colors.borderFocus, }, }); diff --git a/packages/design-system/src/components/__DEPRECATED__/list.tsx b/packages/design-system/src/components/__DEPRECATED__/list.tsx index 00101d9a956..a4317ee9a7e 100644 --- a/packages/design-system/src/components/__DEPRECATED__/list.tsx +++ b/packages/design-system/src/components/__DEPRECATED__/list.tsx @@ -33,7 +33,7 @@ const ListItemBase = styled("li", { pointerEvents: "none", inset: `0 ${theme.spacing[3]}`, borderRadius: theme.borderRadius[4], - border: `2px solid ${theme.colors.borderFocus}`, + border: `1px solid ${theme.colors.borderFocus}`, }, }); diff --git a/packages/design-system/src/components/button.tsx b/packages/design-system/src/components/button.tsx index 19b3ba498f1..f6738a6a4b6 100644 --- a/packages/design-system/src/components/button.tsx +++ b/packages/design-system/src/components/button.tsx @@ -81,7 +81,7 @@ const perColorStyle = (variant: ButtonColor) => ({ "&[data-state=auto]:focus-visible, &[data-state=focus]": { color: foregrounds[variant], - outline: `2px solid ${theme.colors.borderFocus}`, + outline: `1px solid ${theme.colors.borderFocus}`, outlineOffset: "1px", }, diff --git a/packages/design-system/src/components/checkbox.tsx b/packages/design-system/src/components/checkbox.tsx index ebeb1b40558..98bfe355ca1 100644 --- a/packages/design-system/src/components/checkbox.tsx +++ b/packages/design-system/src/components/checkbox.tsx @@ -23,7 +23,7 @@ const checkboxStyle = css({ color: theme.colors.foregroundMain, "&:focus-visible": { - outline: `2px solid ${theme.colors.borderFocus}`, + outline: `1px solid ${theme.colors.borderFocus}`, }, "&[data-state=checked], &[data-state=indeterminate]": { diff --git a/packages/design-system/src/components/component-card.tsx b/packages/design-system/src/components/component-card.tsx index 47e0c387562..c49df9be13d 100644 --- a/packages/design-system/src/components/component-card.tsx +++ b/packages/design-system/src/components/component-card.tsx @@ -20,6 +20,7 @@ const cardStyle = css({ border: `1px solid`, borderColor: theme.colors.borderMain, borderRadius: theme.borderRadius[2], + outline: "none", userSelect: "none", color: theme.colors.foregroundIconMain, cursor: "grab", @@ -32,8 +33,7 @@ const cardStyle = css({ color: theme.colors.foregroundDisabled, }, "&:focus-visible, &[data-state=selected]": { - outline: `2px solid ${theme.colors.borderFocus}`, - outlineOffset: "-2px", + borderColor: theme.colors.borderFocus, }, "& svg": { flexGrow: 0, diff --git a/packages/design-system/src/components/css-value-list-item.tsx b/packages/design-system/src/components/css-value-list-item.tsx index 9dfb91d3b21..4b35dbaafeb 100644 --- a/packages/design-system/src/components/css-value-list-item.tsx +++ b/packages/design-system/src/components/css-value-list-item.tsx @@ -74,17 +74,6 @@ const ItemButton = styled("button", { [`~ ${IconButtonsWrapper}`]: { display: "flex", }, - - "&:after": { - borderRadius: theme.borderRadius[3], - outline: `2px solid ${theme.colors.borderFocus}`, - outlineOffset: "-2px", - position: "absolute", - content: '""', - inset: "0 2px", - pointerEvents: "none", - }, - outline: "none", backgroundColor: theme.colors.backgroundHover, }, diff --git a/packages/design-system/src/components/focus-ring.ts b/packages/design-system/src/components/focus-ring.ts index ff9e0238935..54d69524ae5 100644 --- a/packages/design-system/src/components/focus-ring.ts +++ b/packages/design-system/src/components/focus-ring.ts @@ -1,18 +1,14 @@ -import { theme, type CSS } from "../stitches.config"; +import { theme } from "../stitches.config"; -export const focusRingStyle = (style?: CSS) => ({ +export const focusRingStyle = () => ({ "&::after": { content: '""', position: "absolute", - left: theme.spacing[3], - right: theme.spacing[3], - top: theme.spacing[2], - bottom: theme.spacing[2], - outlineWidth: 2, + inset: theme.spacing[3], + outlineWidth: 1, outlineStyle: "solid", outlineColor: theme.colors.borderFocus, borderRadius: theme.borderRadius[3], pointerEvents: "none", - ...style, }, }); diff --git a/packages/design-system/src/components/icon-button.tsx b/packages/design-system/src/components/icon-button.tsx index e26af728c2d..8ec47299c41 100644 --- a/packages/design-system/src/components/icon-button.tsx +++ b/packages/design-system/src/components/icon-button.tsx @@ -37,10 +37,10 @@ export const IconButton = styled("button", { height: theme.spacing[12], borderRadius: theme.borderRadius[3], minWidth: 0, + outline: "none", "&[data-focused=true], &:focus-visible": { - outline: `2px solid ${theme.colors.borderFocus}`, - outlineOffset: -2, + borderColor: theme.colors.borderFocus, }, "&:disabled, &[aria-disabled=true]": { @@ -65,6 +65,9 @@ export const IconButton = styled("button", { "&:hover, &[data-hovered=true]": openOrHoverStateStyle, }, + "&[data-focused=true], &:focus-visible": { + borderColor: theme.colors.borderFocus, + }, ...disabledVariantStyles, }, diff --git a/packages/design-system/src/components/input-field.tsx b/packages/design-system/src/components/input-field.tsx index 1778b0e19be..7d1dacaa194 100644 --- a/packages/design-system/src/components/input-field.tsx +++ b/packages/design-system/src/components/input-field.tsx @@ -85,15 +85,14 @@ const containerStyle = css({ border: `solid 1px ${theme.colors.borderMain}`, backgroundColor: theme.colors.backgroundControls, "&:focus-within": { - outline: `solid 2px ${theme.colors.borderFocus}`, - outlineOffset: "-1px", + border: `solid 1px ${theme.colors.borderFocus}`, }, "&:has([data-input-field-input][data-color=error])": { borderColor: theme.colors.borderDestructiveMain, }, "&:focus-within:has([data-color=error])": { - outlineColor: theme.colors.borderDestructiveMain, + borderColor: theme.colors.borderDestructiveMain, }, "&:has([data-input-field-input]:is(:disabled, [aria-disabled=true]))": { @@ -102,7 +101,7 @@ const containerStyle = css({ variants: { variant: { chromeless: { - "&:not(:hover)": { + "&:not(:hover, :focus-within)": { borderColor: "transparent", backgroundColor: "transparent", }, diff --git a/packages/design-system/src/components/panel-tabs.tsx b/packages/design-system/src/components/panel-tabs.tsx index e99274a692c..b1882558af0 100644 --- a/packages/design-system/src/components/panel-tabs.tsx +++ b/packages/design-system/src/components/panel-tabs.tsx @@ -32,8 +32,8 @@ export const PanelTabsTrigger = styled(Primitive.Trigger, { }, "&:focus-visible": { - outline: `2px solid ${theme.colors.borderFocus}`, - outlineOffset: "-2px", + outline: `1px solid ${theme.colors.borderFocus}`, + outlineOffset: "-1px", }, "&[data-state=active]": { color: theme.colors.foregroundMain }, diff --git a/packages/design-system/src/components/position-grid.tsx b/packages/design-system/src/components/position-grid.tsx index 86765465b5d..01418aa5315 100644 --- a/packages/design-system/src/components/position-grid.tsx +++ b/packages/design-system/src/components/position-grid.tsx @@ -36,8 +36,8 @@ const containerStyle = css({ padding: theme.spacing[4], width: "fit-content", borderRadius: theme.borderRadius[4], - outline: `1px solid ${theme.colors.borderMain}`, - outlineOffset: "-1px", + border: `1px solid ${theme.colors.borderMain}`, + outline: "none", gridTemplateColumns: "repeat(3, 1fr)", gridTemplateAreas: ` "x x x" @@ -45,14 +45,16 @@ const containerStyle = css({ "x x x" `, "&[data-focused=true], &:focus-visible": { - outline: `2px solid ${theme.colors.borderFocus}`, + borderColor: theme.colors.borderFocus, }, }); const dotStyle = css({ padding: theme.spacing[5], background: theme.colors.backgroundControls, + border: `1px solid transparent`, borderRadius: theme.borderRadius[4], + outline: "none", minWidth: "auto", "&::before": { content: '""', @@ -69,8 +71,7 @@ const dotStyle = css({ }, }, "&[data-focused=true]": { - outline: `2px solid ${theme.colors.borderFocus}`, - outlineOffset: -2, + borderColor: theme.colors.borderFocus, }, }); diff --git a/packages/design-system/src/components/primitives/small-button.tsx b/packages/design-system/src/components/primitives/small-button.tsx index d9abdd1901e..5dfb5680f1d 100644 --- a/packages/design-system/src/components/primitives/small-button.tsx +++ b/packages/design-system/src/components/primitives/small-button.tsx @@ -48,7 +48,7 @@ const perVariantStyle = (variant: (typeof smallButtonVariants)[number]) => ({ }, "&[data-focused=true], &:focus-visible": { borderRadius: theme.borderRadius[3], - outline: `2px solid ${focusColors[variant]}`, + outline: `1px solid ${focusColors[variant]}`, "&:disabled, &[data-disabled]": { outline: "none", }, diff --git a/packages/design-system/src/components/select-button.tsx b/packages/design-system/src/components/select-button.tsx index bb3588a2679..b4cc08688c9 100644 --- a/packages/design-system/src/components/select-button.tsx +++ b/packages/design-system/src/components/select-button.tsx @@ -46,7 +46,6 @@ const style = css({ }, "&:focus-visible": { borderColor: theme.colors.borderFocus, - outline: `1px solid ${theme.colors.borderFocus}`, }, variants: { fullWidth: { true: { width: "100%" } }, diff --git a/packages/design-system/src/components/switch.tsx b/packages/design-system/src/components/switch.tsx index 37d1da0cddf..8a2cd2307da 100644 --- a/packages/design-system/src/components/switch.tsx +++ b/packages/design-system/src/components/switch.tsx @@ -39,7 +39,7 @@ const switchStyle = css({ }, "&:focus": { - outline: `2px solid ${theme.colors.borderFocus}`, + outline: `1px solid ${theme.colors.borderFocus}`, }, }); diff --git a/packages/design-system/src/components/text-area.tsx b/packages/design-system/src/components/text-area.tsx index bb4318a53b1..914ccb77c8e 100644 --- a/packages/design-system/src/components/text-area.tsx +++ b/packages/design-system/src/components/text-area.tsx @@ -28,7 +28,6 @@ const gridStyle = css({ width: "100%", "&:focus-within": { borderColor: theme.colors.borderFocus, - outline: `1px solid ${theme.colors.borderFocus}`, }, "&:has([data-color=error])": { borderColor: theme.colors.borderDestructiveMain, diff --git a/packages/design-system/src/components/toolbar.tsx b/packages/design-system/src/components/toolbar.tsx index 39b6b49e847..778fd14c3e9 100644 --- a/packages/design-system/src/components/toolbar.tsx +++ b/packages/design-system/src/components/toolbar.tsx @@ -24,10 +24,7 @@ export const ToolbarToggleGroup = styled(ToolbarPrimitive.ToggleGroup, { alignItems: "center", }); -const focusRing = focusRingStyle({ - top: theme.spacing[3], - bottom: theme.spacing[3], -}); +const focusRing = focusRingStyle(); const toggleItemStyle = css(textVariants.labelsTitleCase, { // reset styles diff --git a/packages/design-system/src/components/tree.tsx b/packages/design-system/src/components/tree.tsx index 33b7bf06c68..4ece163e1f1 100644 --- a/packages/design-system/src/components/tree.tsx +++ b/packages/design-system/src/components/tree.tsx @@ -100,9 +100,7 @@ const NodeContainer = styled("div", { position: "relative", height: ITEM_HEIGHT, "&:hover, &:has(:focus-visible), &:has([aria-current=true])": { - outline: `var(${treeNodeOutline}, 2px solid ${theme.colors.borderFocus})`, - outlineOffset: -3, - borderRadius: theme.borderRadius[6], + backgroundColor: theme.colors.backgroundHover, [treeActionOpacity]: 1, }, "&:has([aria-selected=true])": {