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

Update Input password component layout to avoid password manager button overlap with visibility button #12398

Merged
merged 5 commits into from
May 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 44 additions & 0 deletions airbyte-webapp/src/components/base/Input/Input.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { render } from "utils/testutils";

import { Input } from "./Input";

describe("<Input />", () => {
test("renders text input", async () => {
const value = "[email protected]";
const { getByTestId, queryByTestId } = await render(<Input type="text" defaultValue={value} />);

expect(getByTestId("input")).toHaveAttribute("type", "text");
expect(getByTestId("input")).toHaveValue(value);
expect(queryByTestId("toggle-password-visibility-button")).toBeFalsy();
});

test("renders another type of input", async () => {
const type = "number";
const value = 888;
const { getByTestId, queryByTestId } = await render(<Input type={type} defaultValue={value} />);

expect(getByTestId("input")).toHaveAttribute("type", type);
expect(getByTestId("input")).toHaveValue(value);
expect(queryByTestId("toggle-password-visibility-button")).toBeFalsy();
});

test("renders password input with visibilty button", async () => {
const value = "eight888";
const { getByTestId, getByRole } = await render(<Input type="password" defaultValue={value} />);

expect(getByTestId("input")).toHaveAttribute("type", "password");
expect(getByTestId("input")).toHaveValue(value);
expect(getByRole("img", { hidden: true })).toHaveAttribute("data-icon", "eye");
});

test("renders visible password when visibility button is clicked", async () => {
const value = "eight888";
const { getByTestId, getByRole } = await render(<Input type="password" defaultValue={value} />);

getByTestId("toggle-password-visibility-button")?.click();

expect(getByTestId("input")).toHaveAttribute("type", "text");
expect(getByTestId("input")).toHaveValue(value);
expect(getByRole("img", { hidden: true })).toHaveAttribute("data-icon", "eye-slash");
});
});
110 changes: 69 additions & 41 deletions airbyte-webapp/src/components/base/Input/Input.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { faEye, faEyeSlash } from "@fortawesome/free-regular-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import React, { useState } from "react";
import React from "react";
import { useIntl } from "react-intl";
import { useToggle } from "react-use";
import styled from "styled-components";
import { Theme } from "theme";

Expand All @@ -18,72 +20,98 @@ const getBackgroundColor = (props: IStyleProps) => {
return props.theme.greyColor0;
};

export type InputProps = {
export interface InputProps extends React.InputHTMLAttributes<HTMLInputElement> {
error?: boolean;
light?: boolean;
} & React.InputHTMLAttributes<HTMLInputElement>;
}

const InputComponent = styled.input<InputProps>`
outline: none;
const InputContainer = styled.div<InputProps>`
width: 100%;
padding: 7px 18px 7px 8px;
border-radius: 4px;
font-size: 14px;
line-height: 20px;
font-weight: normal;
border: 1px solid ${(props) => (props.error ? props.theme.dangerColor : props.theme.greyColor0)};
position: relative;
background: ${(props) => getBackgroundColor(props)};
color: ${({ theme }) => theme.textColor};
caret-color: ${({ theme }) => theme.primaryColor};

&::placeholder {
color: ${({ theme }) => theme.greyColor40};
}
border: 1px solid ${(props) => (props.error ? props.theme.dangerColor : props.theme.greyColor0)};
border-radius: 4px;

&:hover {
background: ${({ theme, light }) => (light ? theme.whiteColor : theme.greyColor20)};
border-color: ${(props) => (props.error ? props.theme.dangerColor : props.theme.greyColor20)};
}

&:focus {
&.input-container--focused {
background: ${({ theme, light }) => (light ? theme.whiteColor : theme.primaryColor12)};
border-color: ${({ theme }) => theme.primaryColor};
}
`;

const InputComponent = styled.input<InputProps & { isPassword?: boolean }>`
outline: none;
width: ${({ isPassword }) => (isPassword ? "calc(100% - 22px)" : "100%")};
padding: 7px 8px 7px 8px;
font-size: 14px;
line-height: 20px;
font-weight: normal;
border: none;
background: none;
color: ${({ theme }) => theme.textColor};
caret-color: ${({ theme }) => theme.primaryColor};

&::placeholder {
color: ${({ theme }) => theme.greyColor40};
}

&:disabled {
pointer-events: none;
color: ${({ theme }) => theme.greyColor55};
}
`;

const Container = styled.div`
width: 100%;
position: relative;
`;

const VisibilityButton = styled(Button)`
position: absolute;
right: 2px;
top: 7px;
right: 0px;
top: 0;
display: flex;
height: 100%;
width: 30px;
align-items: center;
justify-content: center;
border: none;
`;

const Input: React.FC<InputProps> = (props) => {
const [isContentVisible, setIsContentVisible] = useState(false);

if (props.type === "password") {
return (
<Container>
<InputComponent {...props} type={isContentVisible ? "text" : "password"} />
{props.disabled ? null : (
<VisibilityButton iconOnly onClick={() => setIsContentVisible(!isContentVisible)} type="button">
<FontAwesomeIcon icon={isContentVisible ? faEyeSlash : faEye} />
</VisibilityButton>
)}
</Container>
);
}

return <InputComponent {...props} />;
const { formatMessage } = useIntl();
const [isContentVisible, setIsContentVisible] = useToggle(false);
const [focused, toggleFocused] = useToggle(false);

const isPassword = props.type === "password";
const isVisibilityButtonVisible = isPassword && !props.disabled;
const type = isPassword ? (isContentVisible ? "text" : "password") : props.type;
krishnaglick marked this conversation as resolved.
Show resolved Hide resolved
const onInputFocusChange = () => toggleFocused();
krishnaglick marked this conversation as resolved.
Show resolved Hide resolved

return (
<InputContainer {...props} className={focused ? "input-container--focused" : undefined}>
<InputComponent
{...props}
type={type}
isPassword={isPassword}
onFocus={onInputFocusChange}
onBlur={onInputFocusChange}
data-testid="input"
/>
{isVisibilityButtonVisible ? (
<VisibilityButton
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is an icon only button, could we still attach an aria-label to it with "Toggle password visibility" (or the like)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Udpated

iconOnly
onClick={() => setIsContentVisible()}
type="button"
aria-label={formatMessage({
id: `ui.input.${isContentVisible ? "hide" : "show"}Password`,
})}
data-testid="toggle-password-visibility-button"
>
<FontAwesomeIcon icon={isContentVisible ? faEyeSlash : faEye} fixedWidth />
</VisibilityButton>
) : null}
</InputContainer>
);
};

export default Input;
Expand Down
2 changes: 2 additions & 0 deletions airbyte-webapp/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,8 @@
"errorView.notFound": "Resource not found",
"errorView.unknown": "Unknown",

"ui.input.showPassword": "Show password",
"ui.input.hidePassword": "Hide password",
"ui.keyValuePair": "{key}: {value}",
"ui.keyValuePairV2": "{key} ({value})",
"ui.keyValuePairV3": "{key}, {value}",
Expand Down
23 changes: 8 additions & 15 deletions airbyte-webapp/src/utils/testutils.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { act, Queries, render as rtlRender, RenderResult } from "@testing-library/react";
import { History } from "history";
import { act, Queries, queries, render as rtlRender, RenderOptions, RenderResult } from "@testing-library/react";
import React from "react";
import { IntlProvider } from "react-intl";
import { MemoryRouter } from "react-router-dom";
Expand All @@ -9,20 +8,14 @@ import { configContext, defaultConfig } from "config";
import { FeatureService } from "hooks/services/Feature";
import en from "locales/en.json";

export type RenderOptions = {
// optionally pass in a history object to control routes in the test
history?: History;
container?: HTMLElement;
};

type WrapperProps = {
children?: React.ReactNode;
children?: React.ReactElement;
};

export async function render(
ui: React.ReactNode,
renderOptions?: RenderOptions
): Promise<RenderResult<Queries, HTMLElement>> {
export async function render<
Q extends Queries = typeof queries,
Container extends Element | DocumentFragment = HTMLElement
>(ui: React.ReactNode, renderOptions?: RenderOptions<Q, Container>): Promise<RenderResult<Q, Container>> {
krishnaglick marked this conversation as resolved.
Show resolved Hide resolved
function Wrapper({ children }: WrapperProps) {
return (
<TestWrapper>
Expand All @@ -35,9 +28,9 @@ export async function render(
);
}

let renderResult: RenderResult<Queries, HTMLElement>;
let renderResult: RenderResult<Q, Container>;
await act(async () => {
renderResult = await rtlRender(<div>{ui}</div>, { wrapper: Wrapper, ...renderOptions });
renderResult = await rtlRender<Q, Container>(<div>{ui}</div>, { wrapper: Wrapper, ...renderOptions });
});

return renderResult!;
Expand Down