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

feat: Added focus ring for focus visible #37700

Merged
merged 24 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
15e3a88
feat: Added focus ring
albinAppsmith Nov 26, 2024
f303d56
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Nov 26, 2024
cb448a0
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Nov 26, 2024
429e7b1
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Nov 26, 2024
19e0258
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Dec 3, 2024
897ed4f
fix: updated focus outline as none
albinAppsmith Dec 3, 2024
cbf6b75
Merge branch 'ads-v2/focus-ring' of https://github.com/appsmithorg/ap…
albinAppsmith Dec 5, 2024
91a4e73
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Dec 16, 2024
99f4306
fix: focus ring
albinAppsmith Dec 16, 2024
bbed9e0
fix: input focu visible issue
albinAppsmith Dec 17, 2024
04ccdf8
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Dec 17, 2024
cbf01c3
fix: focus issues
albinAppsmith Dec 17, 2024
0707ddd
fix: date puicker focus
albinAppsmith Dec 17, 2024
0b4601e
fix: input focus state
albinAppsmith Dec 17, 2024
bd47af2
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Dec 17, 2024
bf0ef8e
fix: Added focus for codemirror
albinAppsmith Dec 18, 2024
b5595de
fix: flag name change
albinAppsmith Dec 18, 2024
4db67f4
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Dec 18, 2024
f0ed3ff
fix: z-index
albinAppsmith Dec 18, 2024
4f67900
Merge branch 'release' of github.com:appsmithorg/appsmith into ads-v2…
albinAppsmith Jan 10, 2025
60f67c5
Merge branch 'release' of github.com:appsmithorg/appsmith into ads-v2…
albinAppsmith Jan 13, 2025
e7483f4
fix: review comments
albinAppsmith Jan 13, 2025
be0c6b4
fix: button focus visible
albinAppsmith Jan 13, 2025
2597087
fix: link styles
albinAppsmith Jan 13, 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
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ const SearchInputWrapper = styled(InputGroup)<{
text-overflow: ellipsis;
width: 100%;
}

input:focus {
border: 1.2px solid var(--ads-old-color-fern-green);
box-sizing: border-box;
width: 100%;
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}

input:active {
Expand Down
16 changes: 10 additions & 6 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,6 +210,7 @@ export const StyledButton = styled.button<{
UNSAFE_width?: string;
disabled?: boolean;
isIconButton?: boolean;
isFocusVisible?: boolean;
}>`
${Variables}
/* Kind style */
Expand Down Expand Up @@ -269,11 +270,14 @@ export const StyledButton = styled.button<{
}
}

/* Focus styles */
&:focus-visible {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}
${({ isFocusVisible }) =>
isFocusVisible &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is only focus-visible needed for the button?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah we don need focus styles right

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;
}
`}
}
`;
18 changes: 13 additions & 5 deletions app/client/packages/design-system/ads/src/Button/Button.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
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 @@ -16,6 +17,11 @@ 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 @@ -37,21 +43,25 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
...rest
} = props;

// disable button when loading
rest.onClick =
// Replace the direct mutation of rest.onClick with this
const handleClick =
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 @@ -61,9 +71,7 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
{isLoading === true && (
<Spinner
className={ButtonLoadingClassName}
iconProps={{
className: ButtonLoadingIconClassName,
}}
iconProps={SPINNER_ICON_PROPS}
size="md"
/>
)}
Expand Down
22 changes: 10 additions & 12 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,20 +174,13 @@ 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),
&:focus:enabled:not(:read-only) {
&:active:enabled:not(:read-only) {
--input-color-border: var(--ads-v2-colors-control-field-active-border);
}

Expand All @@ -203,11 +196,16 @@ export const StyledInput = styled.input<{
--input-color-border: var(--ads-v2-colors-control-field-error-border);
}

&:active:enabled:not(:read-only),
&:focus:enabled:not(:read-only) {
&:active: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,5 +1,4 @@
import React, { forwardRef } from "react";
import { useFocusRing } from "@react-aria/focus";
import React, { forwardRef, useCallback } from "react";
import { useTextField } from "@react-aria/textfield";
import clsx from "classnames";

Expand Down Expand Up @@ -55,7 +54,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 @@ -67,9 +66,12 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
...restOfEndIconProps
} = endIconProps || {};

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

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

Expand Down Expand Up @@ -116,7 +118,6 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
<StyledInput
as={renderAs}
type={type}
{...focusProps}
{...inputProps}
UNSAFE_height={UNSAFE_height}
UNSAFE_width={UNSAFE_width}
Expand All @@ -126,7 +127,6 @@ 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);
outline-offset: var(--ads-v2-offset-outline);
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
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);
outline-offset: var(--ads-v2-offset-outline);
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
}

&[data-disabled="true"] {
Expand Down
12 changes: 9 additions & 3 deletions app/client/packages/design-system/ads/src/Select/styles.css
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
.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 @@ -51,7 +53,6 @@
.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 @@ -87,6 +88,7 @@
/* 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 @@ -113,6 +115,10 @@
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,8 +72,9 @@ export const StyledSwitchInput = styled.input<{
${({ isFocusVisible }) =>
isFocusVisible &&
`
outline: var(--ads-v2-border-width-outline) solid var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
`}

&: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);
outline-offset: var(--ads-v2-offset-outline);
var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,13 @@ 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: transparent;
--ads-v2-border-width-outline: 0;
--ads-v2-offset-outline: 0;
--ads-v2-color-outline: var(--ads-v2-color-blue-300);
--ads-v2-border-width-outline: 2px;
--ads-v2-offset-outline: -2px;

/**
* ===========================================*
Expand All @@ -251,17 +251,22 @@
--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);
outline-offset: var(--ads-v2-offset-outline);
outline: var(--ads-v2-border-width-outline) solid var(--ads-v2-color-outline) !important;
outline-offset: var(--ads-v2-offset-outline) !important;
}

/* 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,6 +1794,7 @@ 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,6 +54,7 @@ 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 @@ -83,6 +84,17 @@ 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 @@ -390,12 +402,11 @@ export const EditorWrapper = styled.div<{
}
}

&:focus {
&:focus-visible {
.CodeMirror.cm-s-duotone-light {
border-color: ${(props) =>
props.borderLess
? "none"
: "var(--ads-v2-color-border-emphasis-plus)"};
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
outline-offset: var(--ads-v2-offset-outline);
}
}

Expand Down
Loading
Loading