From 9481f263fc9e31b82c13e1fac9ab86f47161691e Mon Sep 17 00:00:00 2001 From: Edmundo Ruiz Ghanem Date: Wed, 27 Apr 2022 09:19:50 -0400 Subject: [PATCH 1/5] Update Input password component visibilty button * Update layout to prevent password manager buttons from overlapping with visiblity button * Apply consistent layout to both password and non-password inputs --- .../src/components/base/Input/Input.tsx | 90 +++++++++++-------- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/airbyte-webapp/src/components/base/Input/Input.tsx b/airbyte-webapp/src/components/base/Input/Input.tsx index 796d12fa880c..f8ece5a49ce4 100644 --- a/airbyte-webapp/src/components/base/Input/Input.tsx +++ b/airbyte-webapp/src/components/base/Input/Input.tsx @@ -1,6 +1,7 @@ import { faEye, faEyeSlash } from "@fortawesome/free-regular-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import React, { useState } from "react"; +import { useToggle } from "react-use"; import styled from "styled-components"; import { Theme } from "theme"; @@ -23,32 +24,39 @@ export type InputProps = { light?: boolean; } & React.InputHTMLAttributes; -const InputComponent = styled.input` - outline: none; +const InputContainer = styled.div` 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` + 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; @@ -56,34 +64,42 @@ const InputComponent = styled.input` } `; -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 = (props) => { const [isContentVisible, setIsContentVisible] = useState(false); - - if (props.type === "password") { - return ( - - - {props.disabled ? null : ( - setIsContentVisible(!isContentVisible)} type="button"> - - - )} - - ); - } - - return ; + const [focused, toggleFocused] = useToggle(false); + + const isPassword = props.type === "password"; + const isVisibilityButtonVisible = isPassword && !props.disabled; + const onInputFocusChange = () => toggleFocused(); + + return ( + + + {isVisibilityButtonVisible ? ( + setIsContentVisible(!isContentVisible)} type="button"> + + + ) : null} + + ); }; export default Input; From 775e990de35fa613fca311ab50c97f1037e2db1f Mon Sep 17 00:00:00 2001 From: Edmundo Ruiz Ghanem Date: Wed, 27 Apr 2022 10:05:59 -0400 Subject: [PATCH 2/5] Add Input component unit test --- .../src/components/base/Input/Input.test.tsx | 34 +++++++++++++++++++ .../src/components/base/Input/Input.tsx | 8 ++++- 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 airbyte-webapp/src/components/base/Input/Input.test.tsx diff --git a/airbyte-webapp/src/components/base/Input/Input.test.tsx b/airbyte-webapp/src/components/base/Input/Input.test.tsx new file mode 100644 index 000000000000..2b70069abcfb --- /dev/null +++ b/airbyte-webapp/src/components/base/Input/Input.test.tsx @@ -0,0 +1,34 @@ +import { render } from "@testing-library/react"; + +import { Input } from "./Input"; + +describe("", () => { + test("renders text input", () => { + const value = "aribyte@example.com"; + const { getByTestId, queryByTestId } = render(); + + expect(getByTestId("input")).toHaveAttribute("type", "text"); + expect(getByTestId("input")).toHaveValue(value); + expect(queryByTestId("toggle-password-visibility-button")).toBeFalsy(); + }); + + test("renders password input with visibilty button", () => { + const value = "eight888"; + const { getByTestId, getByRole } = render(); + + 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", () => { + const value = "eight888"; + const { getByTestId, getByRole } = render(); + + 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"); + }); +}); diff --git a/airbyte-webapp/src/components/base/Input/Input.tsx b/airbyte-webapp/src/components/base/Input/Input.tsx index f8ece5a49ce4..51bd0015be47 100644 --- a/airbyte-webapp/src/components/base/Input/Input.tsx +++ b/airbyte-webapp/src/components/base/Input/Input.tsx @@ -92,9 +92,15 @@ const Input: React.FC = (props) => { isPassword={isPassword} onFocus={onInputFocusChange} onBlur={onInputFocusChange} + data-testid="input" /> {isVisibilityButtonVisible ? ( - setIsContentVisible(!isContentVisible)} type="button"> + setIsContentVisible(!isContentVisible)} + type="button" + data-testid="toggle-password-visibility-button" + > ) : null} From fe51cb3dc97504e46fa86bdd6feb6efc5debca93 Mon Sep 17 00:00:00 2001 From: Edmundo Ruiz Ghanem Date: Wed, 27 Apr 2022 10:51:30 -0400 Subject: [PATCH 3/5] Update InputProps to interface --- airbyte-webapp/src/components/base/Input/Input.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/airbyte-webapp/src/components/base/Input/Input.tsx b/airbyte-webapp/src/components/base/Input/Input.tsx index 51bd0015be47..694f72face40 100644 --- a/airbyte-webapp/src/components/base/Input/Input.tsx +++ b/airbyte-webapp/src/components/base/Input/Input.tsx @@ -19,10 +19,10 @@ const getBackgroundColor = (props: IStyleProps) => { return props.theme.greyColor0; }; -export type InputProps = { +export interface InputProps extends React.InputHTMLAttributes { error?: boolean; light?: boolean; -} & React.InputHTMLAttributes; +} const InputContainer = styled.div` width: 100%; From 2c47718a4ac0562f3780b8de5f959d67d58b7c5f Mon Sep 17 00:00:00 2001 From: Edmundo Ruiz Ghanem Date: Wed, 27 Apr 2022 11:18:07 -0400 Subject: [PATCH 4/5] Ensure input component can be assigned type --- .../src/components/base/Input/Input.test.tsx | 12 +++++++++++- airbyte-webapp/src/components/base/Input/Input.tsx | 3 ++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/airbyte-webapp/src/components/base/Input/Input.test.tsx b/airbyte-webapp/src/components/base/Input/Input.test.tsx index 2b70069abcfb..8377b6566622 100644 --- a/airbyte-webapp/src/components/base/Input/Input.test.tsx +++ b/airbyte-webapp/src/components/base/Input/Input.test.tsx @@ -5,13 +5,23 @@ import { Input } from "./Input"; describe("", () => { test("renders text input", () => { const value = "aribyte@example.com"; - const { getByTestId, queryByTestId } = render(); + const { getByTestId, queryByTestId } = render(); expect(getByTestId("input")).toHaveAttribute("type", "text"); expect(getByTestId("input")).toHaveValue(value); expect(queryByTestId("toggle-password-visibility-button")).toBeFalsy(); }); + test("renders other type of input", () => { + const type = "number"; + const value = 888; + const { getByTestId, queryByTestId } = render(); + + 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", () => { const value = "eight888"; const { getByTestId, getByRole } = render(); diff --git a/airbyte-webapp/src/components/base/Input/Input.tsx b/airbyte-webapp/src/components/base/Input/Input.tsx index 694f72face40..62a40a873886 100644 --- a/airbyte-webapp/src/components/base/Input/Input.tsx +++ b/airbyte-webapp/src/components/base/Input/Input.tsx @@ -82,13 +82,14 @@ const Input: React.FC = (props) => { const isPassword = props.type === "password"; const isVisibilityButtonVisible = isPassword && !props.disabled; + const type = isPassword ? (isContentVisible ? "text" : "password") : props.type; const onInputFocusChange = () => toggleFocused(); return ( Date: Wed, 27 Apr 2022 14:37:37 -0400 Subject: [PATCH 5/5] Add aria label to visiblity button in password input * Fix type issues with testutils render --- .../src/components/base/Input/Input.test.tsx | 20 ++++++++-------- .../src/components/base/Input/Input.tsx | 11 ++++++--- airbyte-webapp/src/locales/en.json | 2 ++ airbyte-webapp/src/utils/testutils.tsx | 23 +++++++------------ 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/airbyte-webapp/src/components/base/Input/Input.test.tsx b/airbyte-webapp/src/components/base/Input/Input.test.tsx index 8377b6566622..d41105391223 100644 --- a/airbyte-webapp/src/components/base/Input/Input.test.tsx +++ b/airbyte-webapp/src/components/base/Input/Input.test.tsx @@ -1,41 +1,41 @@ -import { render } from "@testing-library/react"; +import { render } from "utils/testutils"; import { Input } from "./Input"; describe("", () => { - test("renders text input", () => { + test("renders text input", async () => { const value = "aribyte@example.com"; - const { getByTestId, queryByTestId } = render(); + const { getByTestId, queryByTestId } = await render(); expect(getByTestId("input")).toHaveAttribute("type", "text"); expect(getByTestId("input")).toHaveValue(value); expect(queryByTestId("toggle-password-visibility-button")).toBeFalsy(); }); - test("renders other type of input", () => { + test("renders another type of input", async () => { const type = "number"; const value = 888; - const { getByTestId, queryByTestId } = render(); + const { getByTestId, queryByTestId } = await render(); 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", () => { + test("renders password input with visibilty button", async () => { const value = "eight888"; - const { getByTestId, getByRole } = render(); + const { getByTestId, getByRole } = await render(); 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", () => { + test("renders visible password when visibility button is clicked", async () => { const value = "eight888"; - const { getByTestId, getByRole } = render(); + const { getByTestId, getByRole } = await render(); - getByTestId("toggle-password-visibility-button").click(); + getByTestId("toggle-password-visibility-button")?.click(); expect(getByTestId("input")).toHaveAttribute("type", "text"); expect(getByTestId("input")).toHaveValue(value); diff --git a/airbyte-webapp/src/components/base/Input/Input.tsx b/airbyte-webapp/src/components/base/Input/Input.tsx index 62a40a873886..117ff76e206a 100644 --- a/airbyte-webapp/src/components/base/Input/Input.tsx +++ b/airbyte-webapp/src/components/base/Input/Input.tsx @@ -1,6 +1,7 @@ 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"; @@ -77,7 +78,8 @@ const VisibilityButton = styled(Button)` `; const Input: React.FC = (props) => { - const [isContentVisible, setIsContentVisible] = useState(false); + const { formatMessage } = useIntl(); + const [isContentVisible, setIsContentVisible] = useToggle(false); const [focused, toggleFocused] = useToggle(false); const isPassword = props.type === "password"; @@ -98,8 +100,11 @@ const Input: React.FC = (props) => { {isVisibilityButtonVisible ? ( setIsContentVisible(!isContentVisible)} + onClick={() => setIsContentVisible()} type="button" + aria-label={formatMessage({ + id: `ui.input.${isContentVisible ? "hide" : "show"}Password`, + })} data-testid="toggle-password-visibility-button" > diff --git a/airbyte-webapp/src/locales/en.json b/airbyte-webapp/src/locales/en.json index 2c1378b5b4ad..9844a505e666 100644 --- a/airbyte-webapp/src/locales/en.json +++ b/airbyte-webapp/src/locales/en.json @@ -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}", diff --git a/airbyte-webapp/src/utils/testutils.tsx b/airbyte-webapp/src/utils/testutils.tsx index ab6a2f354d14..9f9e50e70fc9 100644 --- a/airbyte-webapp/src/utils/testutils.tsx +++ b/airbyte-webapp/src/utils/testutils.tsx @@ -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"; @@ -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> { +export async function render< + Q extends Queries = typeof queries, + Container extends Element | DocumentFragment = HTMLElement +>(ui: React.ReactNode, renderOptions?: RenderOptions): Promise> { function Wrapper({ children }: WrapperProps) { return ( @@ -35,9 +28,9 @@ export async function render( ); } - let renderResult: RenderResult; + let renderResult: RenderResult; await act(async () => { - renderResult = await rtlRender(
{ui}
, { wrapper: Wrapper, ...renderOptions }); + renderResult = await rtlRender(
{ui}
, { wrapper: Wrapper, ...renderOptions }); }); return renderResult!;