Skip to content

Commit

Permalink
Revert "feat: Added focus ring for focus visible (#37700)" (#38650)
Browse files Browse the repository at this point in the history
  • Loading branch information
hetunandu authored Jan 14, 2025
1 parent 3ad06e2 commit e1b3b0d
Show file tree
Hide file tree
Showing 18 changed files with 81 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,10 @@ const SearchInputWrapper = styled(InputGroup)<{
text-overflow: ellipsis;
width: 100%;
}
input:focus {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
border: 1.2px solid var(--ads-old-color-fern-green);
box-sizing: border-box;
width: 100%;
}
input:active {
Expand Down
16 changes: 6 additions & 10 deletions app/client/packages/design-system/ads/src/Button/Button.styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ export const StyledButton = styled.button<{
UNSAFE_width?: string;
disabled?: boolean;
isIconButton?: boolean;
isFocusVisible?: boolean;
}>`
${Variables}
/* Kind style */
Expand Down Expand Up @@ -270,14 +269,11 @@ export const StyledButton = styled.button<{
}
}
${({ isFocusVisible }) =>
isFocusVisible &&
css`
:focus-visible {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
}
`}
/* Focus styles */
&:focus-visible {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}
}
`;
18 changes: 5 additions & 13 deletions app/client/packages/design-system/ads/src/Button/Button.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { forwardRef } from "react";
import clsx from "classnames";
import { useFocusRing } from "@react-aria/focus";

import { StyledButton, ButtonContent } from "./Button.styles";
import type { ButtonProps } from "./Button.types";
Expand All @@ -17,11 +16,6 @@ import {
} from "./Button.constants";
import { Spinner } from "../Spinner";

// Add this before the Button component definition
const SPINNER_ICON_PROPS = {
className: ButtonLoadingIconClassName,
};

/**
* TODO:
* - if both left and right icon is used, disregard left icon and print a warning in the console.
Expand All @@ -43,25 +37,21 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
...rest
} = props;

// Replace the direct mutation of rest.onClick with this
const handleClick =
// disable button when loading
rest.onClick =
props.isLoading || props.isDisabled ? undefined : props.onClick;
const buttonRef = useDOMRef(ref);
const { focusProps, isFocusVisible } = useFocusRing();

return (
<StyledButton
as={renderAs || "button"}
{...rest}
onClick={handleClick}
{...focusProps}
UNSAFE_height={UNSAFE_height}
UNSAFE_width={UNSAFE_width}
className={clsx(ButtonClassName, className)}
data-disabled={props.isDisabled || false}
data-loading={isLoading}
disabled={props.isDisabled}
isFocusVisible={isFocusVisible}
isIconButton={isIconButton}
kind={kind}
ref={buttonRef}
Expand All @@ -71,7 +61,9 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
{isLoading === true && (
<Spinner
className={ButtonLoadingClassName}
iconProps={SPINNER_ICON_PROPS}
iconProps={{
className: ButtonLoadingIconClassName,
}}
size="md"
/>
)}
Expand Down
22 changes: 12 additions & 10 deletions app/client/packages/design-system/ads/src/Input/Input.styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export const StyledInput = styled.input<{
renderer === "input" &&
css`
padding-left: calc((var(--input-padding-x) * 2) + var(--icon-size) - 1px);
`}
`};
/* adjust padding end according to icon present or not */
${({ hasEndIcon, renderer }) =>
Expand All @@ -174,13 +174,20 @@ export const StyledInput = styled.input<{
padding-right: calc(
(var(--input-padding-x) * 2) + var(--icon-size) - 1px
);
`}
`};
&:focus:enabled:not(:read-only) {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}
&:hover:enabled:not(:read-only) {
--input-color-border: var(--ads-v2-colors-control-field-hover-border);
}
&:active:enabled:not(:read-only) {
&:active:enabled:not(:read-only),
&:focus:enabled:not(:read-only) {
--input-color-border: var(--ads-v2-colors-control-field-active-border);
}
Expand All @@ -196,16 +203,11 @@ export const StyledInput = styled.input<{
--input-color-border: var(--ads-v2-colors-control-field-error-border);
}
&:active:enabled:not(:read-only) {
&:active:enabled:not(:read-only),
&:focus:enabled:not(:read-only) {
--input-color-border: var(--ads-v2-colors-control-field-error-border);
}
}
&:focus {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
}
`;

export const Description = styled(Text)`
Expand Down
16 changes: 8 additions & 8 deletions app/client/packages/design-system/ads/src/Input/Input.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { forwardRef, useCallback } from "react";
import React, { forwardRef } from "react";
import { useFocusRing } from "@react-aria/focus";
import { useTextField } from "@react-aria/textfield";
import clsx from "classnames";

Expand Down Expand Up @@ -54,7 +55,7 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
const { descriptionProps, errorMessageProps, inputProps, labelProps } =
// @ts-expect-error fix this the next time the file is edited
useTextField(props, inputRef);

const { focusProps, isFocusVisible } = useFocusRing();
const {
className: startIconClassName,
onClick: startIconOnClick,
Expand All @@ -66,12 +67,9 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
...restOfEndIconProps
} = endIconProps || {};

const handleOnChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
onChange?.(event.target.value);
},
[onChange],
);
const handleOnChange = (event: React.ChangeEvent<HTMLInputElement>) => {
onChange?.(event.target.value);
};

isValid = isValid === undefined ? !errorMessage : isValid;

Expand Down Expand Up @@ -118,6 +116,7 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
<StyledInput
as={renderAs}
type={type}
{...focusProps}
{...inputProps}
UNSAFE_height={UNSAFE_height}
UNSAFE_width={UNSAFE_width}
Expand All @@ -127,6 +126,7 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
hasEndIcon={!!endIcon}
hasStartIcon={!!startIcon}
inputSize={size}
isFocusVisible={isFocusVisible}
onChange={handleOnChange}
readOnly={isReadOnly}
ref={inputRef}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ export const Styles = css<{ kind?: LinkKind }>`
:focus-visible {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
border-radius: var(--ads-v2-border-radius);
}
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ export const StyledControlContainer = styled.div`
&:focus-visible {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}
&[data-disabled="true"] {
Expand Down
12 changes: 3 additions & 9 deletions app/client/packages/design-system/ads/src/Select/styles.css
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
.ads-v2-select {
--select-color-border: var(--ads-v2-colors-control-field-default-border);
--select-padding-x: var(--ads-v2-spaces-2);
/* padding left and right */
--select-padding-y: var(--ads-v2-spaces-2);
/* padding top and bottom */
--select-padding-x: var(--ads-v2-spaces-2); /* padding left and right */
--select-padding-y: var(--ads-v2-spaces-2); /* padding top and bottom */
--select-font-size: var(--ads-v2-font-size-2);
--select-height: 24px;
--select-search-input-padding-right: 0;
Expand Down Expand Up @@ -53,6 +51,7 @@
.ads-v2-select.rc-select-focused {
outline: var(--ads-v2-border-width-outline) solid var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
--select-color-border: var(--ads-v2-colors-control-field-active-border);
}

/* Error */
Expand Down Expand Up @@ -88,7 +87,6 @@
/* padding x + icon size + gap */
--select-search-input-padding-right: calc(var(--select-padding-x) + 16px);
}

.ads-v2-select.rc-select-allow-clear.rc-select-show-arrow.rc-select-selection-search-input,
.ads-v2-select.rc-select-allow-clear.rc-select-show-arrow
.rc-select-selection-overflow {
Expand All @@ -115,10 +113,6 @@
overflow: hidden;
}

.ads-v2-select.rc-select-focused > .rc-select-selector {
border-color: transparent;
}

/* Placeholder */
.ads-v2-select > .rc-select-selector > .rc-select-selection-placeholder,
.ads-v2-select > .rc-select-selector > .rc-select-selection-item {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,8 @@ export const StyledSwitchInput = styled.input<{
${({ isFocusVisible }) =>
isFocusVisible &&
`
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
outline: var(--ads-v2-border-width-outline) solid var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
`}
&:hover {
Expand Down
4 changes: 2 additions & 2 deletions app/client/packages/design-system/ads/src/Tab/Tab.styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export const StyledTab = styled(RadixTabs.TabsTrigger)`
&:focus-visible {
--tab-color: var(--ads-v2-colors-content-label-default-fg);
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,8 @@ export const StyledEditableInput = styled.input`
border-color: var(--ads-v2-colors-control-field-hover-border);
}
&:focus,
&:active {
border-color: var(--ads-v2-colors-control-field-active-border);
}
&:focus-visible {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,9 @@
/* --ads-v2-color-outline: var(--ads-v2-color-blue-300);
--ads-v2-border-width-outline: 2px; */
/* TODO: fix focus issue across the platform */
--ads-v2-color-outline: var(--ads-v2-color-blue-300);
--ads-v2-border-width-outline: 2px;
--ads-v2-offset-outline: -2px;
--ads-v2-color-outline: transparent;
--ads-v2-border-width-outline: 0;
--ads-v2-offset-outline: 0;

/**
* ===========================================*
Expand All @@ -251,22 +251,17 @@
--ads-v2-opacity-disabled: 0.6;
}

:focus {
outline: none !important;
}

/* react-aria useFocusRing helper class*/
.ads-v2-focus-ring {
outline: var(--ads-v2-border-width-outline) solid var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
outline: var(--ads-v2-border-width-outline) solid var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}

/* Placeholder color */
::placeholder {
/* This needs to be important to override the placeholder color on main repo */
color: var(--ads-v2-colors-control-placeholder-default-fg) !important;
opacity: 1;
/* Firefox */
opacity: 1; /* Firefox */
}

:-ms-input-placeholder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1794,7 +1794,6 @@ class CodeEditor extends Component<Props, State> {
onMouseOver={this.handleMouseMove}
ref={this.editorWrapperRef}
removeHoverAndFocusStyle={this.props?.removeHoverAndFocusStyle}
showFocusVisible={!this.props.isJSObject}
size={size}
>
{this.state.peekOverlayProps && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export const EditorWrapper = styled.div<{
AIEnabled?: boolean;
mode: string;
maxHeight?: string | number;
showFocusVisible?: boolean;
}>`
// Bottom border was getting clipped
.CodeMirror.cm-s-duotone-light.CodeMirror-wrap {
Expand Down Expand Up @@ -84,17 +83,6 @@ export const EditorWrapper = styled.div<{
text-transform: none;
&& {
${(props) =>
props.showFocusVisible &&
`
.CodeMirror-focused {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
z-index: 1 !important;
}
`}
.CodeMirror-cursor {
border-right: none;
border-left-width: 2px;
Expand Down Expand Up @@ -402,11 +390,12 @@ export const EditorWrapper = styled.div<{
}
}
&:focus-visible {
&:focus {
.CodeMirror.cm-s-duotone-light {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
border-color: ${(props) =>
props.borderLess
? "none"
: "var(--ads-v2-color-border-emphasis-plus)"};
}
}
Expand Down
Loading

0 comments on commit e1b3b0d

Please sign in to comment.