From fa22a5e7ad274e5be0baf32ef0f29918a3a733fa Mon Sep 17 00:00:00 2001 From: Johannes Kettmann Date: Mon, 5 Dec 2022 16:05:08 +0100 Subject: [PATCH 1/8] Refactor select --- .../issues/components/filters/filters.tsx | 51 ++++++++-------- features/ui/form/select/index.ts | 3 +- features/ui/form/select/option.tsx | 16 ++--- features/ui/form/select/select.tsx | 58 +++++++++++-------- features/ui/form/select/selectContext.ts | 15 ++--- 5 files changed, 73 insertions(+), 70 deletions(-) diff --git a/features/issues/components/filters/filters.tsx b/features/issues/components/filters/filters.tsx index 07a0f26..eb4c331 100644 --- a/features/issues/components/filters/filters.tsx +++ b/features/issues/components/filters/filters.tsx @@ -5,6 +5,7 @@ import React, { useRef, useContext, } from "react"; +import { useRouter } from "next/router"; import styled from "styled-components"; import { Select, @@ -14,12 +15,25 @@ import { IconOptions, NavigationContext, } from "@features/ui"; -import { useFilters, IssueLevel, IssueStatus } from "@features/issues"; +import { useFilters } from "../../hooks"; +import { IssueLevel, IssueStatus } from "../../types/issue.types"; import { useProjects } from "@features/projects"; import { breakpoint } from "@styles/theme"; import { useWindowSize } from "react-use"; +import { OptionType } from "@features/ui/form/select/select"; -import { useRouter } from "next/router"; +const statusOptions = [ + { value: undefined, text: "--None--" }, + { value: IssueStatus.open, text: "Unresolved" }, + { value: IssueStatus.resolved, text: "Resolved" }, +] as OptionType[]; + +const levelOptions = [ + { value: undefined, text: "--None--" }, + { value: IssueLevel.error, text: "Error" }, + { value: IssueLevel.warning, text: "Warning" }, + { value: IssueLevel.info, text: "Info" }, +] as OptionType[]; const Container = styled.div` display: flex; @@ -148,6 +162,7 @@ export function Filters() { placeholder="Status" defaultValue="Status" width={isMobileScreen ? "97%" : "8rem"} + options={statusOptions} data-cy="filter-by-status" style={{ ...(isMobileMenuOpen && { @@ -155,21 +170,18 @@ export function Filters() { }), }} > - - - + {statusOptions.map((option) => ( + + ))} unknown; }; const ListItem = styled.li.attrs(() => ({ @@ -42,20 +41,15 @@ const ListItemIcon = styled.img<{ isCurrentlySelected: boolean }>` height: ${space(4)}; `; -export function Option({ children, value, handleCallback }: OptionProps) { - const { changeSelectedOption, selectedOption } = useSelectContext(); - const isCurrentlySelected = selectedOption === value; +export function Option({ children, value }: OptionProps) { + const { changeSelectedValue, selectedValue } = useSelectContext(); + const isCurrentlySelected = selectedValue === value; return ( { - changeSelectedOption(value); - if (handleCallback) { - handleCallback(value); - } - }} + onClick={() => changeSelectedValue(value)} role="option" > {children} diff --git a/features/ui/form/select/select.tsx b/features/ui/form/select/select.tsx index 5c23288..8d5ca4c 100644 --- a/features/ui/form/select/select.tsx +++ b/features/ui/form/select/select.tsx @@ -1,8 +1,6 @@ import React, { useState, ReactNode, - useCallback, - useMemo, useRef, SelectHTMLAttributes, } from "react"; @@ -11,7 +9,12 @@ import { useClickAway } from "react-use"; import { SelectContext } from "./selectContext"; import { color, textFont, space } from "@styles/theme"; -type SelectProps = SelectHTMLAttributes & { +export type OptionType = { + text: string; + value: string; +}; + +type SelectProps = Omit, "onChange"> & { children: ReactNode | ReactNode[]; errorMessage?: string; defaultValue?: string; @@ -21,6 +24,9 @@ type SelectProps = SelectHTMLAttributes & { width?: string | number; label?: string; hint?: string; + value?: string; + onChange?: (value?: string) => void; + options: OptionType[]; }; const Container = styled.div` @@ -129,17 +135,19 @@ const ErrorMessage = styled.p` export function Select({ placeholder = "Choose an option", - defaultValue = "", - iconSrc = "", + defaultValue, + value, + iconSrc, disabled = false, - label = "", - hint = "", - errorMessage = "", - width = "", + label, + hint, + errorMessage, + width, children, + options, + onChange, ...props }: SelectProps) { - const [selectedOption, setSelectedOption] = useState(defaultValue || ""); const [showDropdown, setShowDropdown] = useState(false); const ref = useRef(null); @@ -148,28 +156,28 @@ export function Select({ setShowDropdown(false); }); - const showDropdownHandler = useCallback( - () => setShowDropdown((prevShowDropdown) => !prevShowDropdown), - [] - ); - - const updateSelectedOption = useCallback((option: string) => { - setSelectedOption(option); + const updateSelectedOption = (value: string) => { + if (onChange) onChange(value); setShowDropdown(false); - }, []); + }; - const value = useMemo( - () => ({ selectedOption, changeSelectedOption: updateSelectedOption }), - [selectedOption, updateSelectedOption] - ); + const selectedOption = + value === undefined + ? undefined + : options.find((option) => option.value === value); return ( - + {label && } setShowDropdown(!showDropdown)} selectedOption={selectedOption} disabled={disabled} errorMessage={errorMessage} @@ -178,7 +186,7 @@ export function Select({ > {iconSrc && } - {selectedOption || placeholder} + {selectedOption?.text || placeholder} void; + selectedValue?: string; + changeSelectedValue: (value: string) => void; }>({ - selectedOption: "", - // eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars - changeSelectedOption: (option: string) => {}, + selectedValue: undefined, + changeSelectedValue: () => null, }); export const useSelectContext = () => { - const context = useContext(SelectContext); - if (!context) { - throw new Error("Error in creating the context"); - } - return context; + return useContext(SelectContext); }; From 455c7bcf197be072479dcc2d39dc199e1913c40f Mon Sep 17 00:00:00 2001 From: Johannes Kettmann Date: Mon, 5 Dec 2022 16:27:33 +0100 Subject: [PATCH 2/8] Use controlled select input --- features/issues/components/filters/filters.tsx | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/features/issues/components/filters/filters.tsx b/features/issues/components/filters/filters.tsx index eb4c331..b3335f0 100644 --- a/features/issues/components/filters/filters.tsx +++ b/features/issues/components/filters/filters.tsx @@ -94,19 +94,10 @@ export function Filters() { }; const handleLevel = (level?: string) => { - if (level) { - level = level.toLowerCase(); - } handleFilters({ level: level as IssueLevel }); }; const handleStatus = (status?: string) => { - if (status === "Unresolved") { - status = "open"; - } - if (status) { - status = status.toLowerCase(); - } handleFilters({ status: status as IssueStatus }); }; @@ -160,8 +151,9 @@ export function Filters() { = 10.0.0" + }, + "peerDependencies": { + "react": ">=16.8.0" + } + }, "node_modules/use-isomorphic-layout-effect": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/use-isomorphic-layout-effect/-/use-isomorphic-layout-effect-1.1.1.tgz", @@ -45627,6 +45639,12 @@ "dev": true, "requires": {} }, + "use-debounce": { + "version": "9.0.2", + "resolved": "https://registry.npmjs.org/use-debounce/-/use-debounce-9.0.2.tgz", + "integrity": "sha512-QLyB0sxt9F5AisGDrUybCRJSLE60bTQR0yXc+IebNGUu1GCXwii1zsZl82mPGdWqDVQy7+1FKMLHQUixxf5Nbw==", + "requires": {} + }, "use-isomorphic-layout-effect": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/use-isomorphic-layout-effect/-/use-isomorphic-layout-effect-1.1.1.tgz", diff --git a/package.json b/package.json index 839cf7b..28f1d8d 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,8 @@ "react-query": "^3.34.19", "react-use": "^17.4.0", "styled-components": "^5.3.3", - "styled-normalize": "^8.0.7" + "styled-normalize": "^8.0.7", + "use-debounce": "^9.0.2" }, "devDependencies": { "@babel/core": "^7.17.5", From fc326c7a079597c0e523357abc2937f3cd99b8aa Mon Sep 17 00:00:00 2001 From: Johannes Kettmann Date: Tue, 6 Dec 2022 09:34:59 +0100 Subject: [PATCH 5/8] Fix z-index workaround --- .../issues/components/filters/filters.tsx | 27 +++---------------- .../sidebar-navigation/sidebar-navigation.tsx | 1 + 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/features/issues/components/filters/filters.tsx b/features/issues/components/filters/filters.tsx index 89b3c03..472d00c 100644 --- a/features/issues/components/filters/filters.tsx +++ b/features/issues/components/filters/filters.tsx @@ -1,14 +1,7 @@ -import React, { useContext, useState } from "react"; +import React, { useState } from "react"; import styled from "styled-components"; import { useDebouncedCallback } from "use-debounce"; -import { - Select, - Option, - Input, - Button, - IconOptions, - NavigationContext, -} from "@features/ui"; +import { Select, Option, Input, Button, IconOptions } from "@features/ui"; import { useFilters } from "../../hooks"; import { IssueLevel, IssueStatus } from "../../types/issue.types"; import { breakpoint } from "@styles/theme"; @@ -60,7 +53,6 @@ export function Filters() { const { updateFilters, filters } = useFilters(); const { width } = useWindowSize(); const isMobileScreen = width <= 1023; - const { isMobileMenuOpen } = useContext(NavigationContext); const [project, setProject] = useState(filters.project); const debouncedUpdateFilters = useDebouncedCallback(updateFilters, 300); @@ -100,11 +92,6 @@ export function Filters() { onChange={handleStatus} options={statusOptions} data-cy="filter-by-status" - style={{ - ...(isMobileMenuOpen && { - opacity: 0, - }), - }} > {statusOptions.map((option) => ( diff --git a/features/ui/sidebar-navigation/sidebar-navigation.tsx b/features/ui/sidebar-navigation/sidebar-navigation.tsx index b7816df..1f3db95 100644 --- a/features/ui/sidebar-navigation/sidebar-navigation.tsx +++ b/features/ui/sidebar-navigation/sidebar-navigation.tsx @@ -48,6 +48,7 @@ const Container = styled.div<{ isCollapsed: boolean }>` const FixedContainer = styled.div` ${containerStyles} position: fixed; + z-index: 1; `; const Header = styled.header` From 44e82df8b6dc6a5e34c942e1311b1c39a6a311d7 Mon Sep 17 00:00:00 2001 From: Johannes Kettmann Date: Tue, 6 Dec 2022 12:23:12 +0100 Subject: [PATCH 6/8] Remove filters context to resolve circular dependency --- features/issues/context/filters-context.tsx | 45 ------------------- features/issues/context/index.ts | 1 - features/issues/index.ts | 1 - features/ui/page-container/page-container.tsx | 31 ++++++------- pages/dashboard/issues.tsx | 15 +++---- 5 files changed, 20 insertions(+), 73 deletions(-) delete mode 100644 features/issues/context/filters-context.tsx delete mode 100644 features/issues/context/index.ts diff --git a/features/issues/context/filters-context.tsx b/features/issues/context/filters-context.tsx deleted file mode 100644 index 0fbf1cc..0000000 --- a/features/issues/context/filters-context.tsx +++ /dev/null @@ -1,45 +0,0 @@ -import React, { - useState, - useMemo, - useCallback, - createContext, - ReactNode, -} from "react"; -import { IssueFilters } from "@features/issues"; - -export const FiltersContext = createContext<{ - filters: IssueFilters; - handleFilters: (filter: IssueFilters) => unknown; -}>({ - filters: { status: undefined, level: undefined, project: undefined }, - // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-empty-function - handleFilters: (_filter: IssueFilters) => {}, -}); - -type FiltersProviderProps = { - children: ReactNode | ReactNode[]; -}; - -export function FiltersProvider({ children }: FiltersProviderProps) { - const [filters, setFilters] = useState({ - status: undefined, - level: undefined, - project: undefined, - }); - - const handleFilters = useCallback( - (filter) => setFilters((prevFilters) => ({ ...prevFilters, ...filter })), - [] - ); - - const memoizedValue = useMemo( - () => ({ filters, handleFilters }), - [filters, handleFilters] - ); - - return ( - - {children} - - ); -} diff --git a/features/issues/context/index.ts b/features/issues/context/index.ts deleted file mode 100644 index 8c4cc1d..0000000 --- a/features/issues/context/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { FiltersContext, FiltersProvider } from "./filters-context"; diff --git a/features/issues/index.ts b/features/issues/index.ts index 3c0c107..3b832e8 100644 --- a/features/issues/index.ts +++ b/features/issues/index.ts @@ -1,5 +1,4 @@ export * from "./api/use-issues"; export * from "./components/IssueList"; export * from "./types/issue.types"; -export * from "./context"; export * from "./hooks"; diff --git a/features/ui/page-container/page-container.tsx b/features/ui/page-container/page-container.tsx index 71c5a91..2d79014 100644 --- a/features/ui/page-container/page-container.tsx +++ b/features/ui/page-container/page-container.tsx @@ -3,7 +3,6 @@ import Head from "next/head"; import styled from "styled-components"; import { SidebarNavigation, Footer } from "@features/ui"; import { color, displayFont, space, breakpoint, textFont } from "@styles/theme"; -import { FiltersProvider } from "@features/issues"; type PageContainerProps = { children: React.ReactNode; @@ -60,23 +59,21 @@ const Info = styled.div` export function PageContainer({ children, title, info }: PageContainerProps) { return ( - - - ProLog - {title} - - - + + ProLog - {title} + + + - -
- - {title} - {info} - {children} - -
-
-
+ +
+ + {title} + {info} + {children} + +
+
); } diff --git a/pages/dashboard/issues.tsx b/pages/dashboard/issues.tsx index 0281713..73a91ce 100644 --- a/pages/dashboard/issues.tsx +++ b/pages/dashboard/issues.tsx @@ -1,18 +1,15 @@ import { PageContainer } from "@features/ui"; import { IssueList } from "@features/issues"; import type { NextPage } from "next"; -import { FiltersProvider } from "@features/issues"; const IssuesPage: NextPage = () => { return ( - - - - - + + + ); }; From ecb6b335e21b4910e80037956ee70cdbdf6761dd Mon Sep 17 00:00:00 2001 From: Johannes Kettmann Date: Tue, 6 Dec 2022 12:25:03 +0100 Subject: [PATCH 7/8] Replace programmatic styles --- .../issues/components/filters/filters.tsx | 48 ++++++++++++------- features/ui/form/input/input.tsx | 1 - features/ui/form/select/select.stories.tsx | 1 - features/ui/form/select/select.tsx | 40 ++++++---------- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/features/issues/components/filters/filters.tsx b/features/issues/components/filters/filters.tsx index 472d00c..78640a6 100644 --- a/features/issues/components/filters/filters.tsx +++ b/features/issues/components/filters/filters.tsx @@ -1,11 +1,16 @@ import React, { useState } from "react"; import styled from "styled-components"; import { useDebouncedCallback } from "use-debounce"; -import { Select, Option, Input, Button, IconOptions } from "@features/ui"; +import { + Select as UnstyledSelect, + Option, + Button as UnstyledButton, + IconOptions, + Input as UnstyledInput, +} from "@features/ui"; import { useFilters } from "../../hooks"; import { IssueLevel, IssueStatus } from "../../types/issue.types"; import { breakpoint } from "@styles/theme"; -import { useWindowSize } from "react-use"; import { OptionType } from "@features/ui/form/select/select"; const statusOptions = [ @@ -21,6 +26,16 @@ const levelOptions = [ { value: IssueLevel.info, text: "Info" }, ] as OptionType[]; +const Button = styled(UnstyledButton)` + height: 2.5rem; + min-width: 8rem; + width: 100%; + + @media (min-width: ${breakpoint("desktop")}) { + width: auto; + } +`; + const Container = styled.div` display: flex; flex-direction: column; @@ -49,11 +64,21 @@ const RightContainer = styled.div` } `; +const Select = styled(UnstyledSelect)` + width: 100%; + + @media (min-width: ${breakpoint("desktop")}) { + width: 8rem; + } +`; + +const Input = styled(UnstyledInput)` + width: 100%; + box-sizing: border-box; +`; + export function Filters() { const { updateFilters, filters } = useFilters(); - const { width } = useWindowSize(); - const isMobileScreen = width <= 1023; - const [project, setProject] = useState(filters.project); const debouncedUpdateFilters = useDebouncedCallback(updateFilters, 300); @@ -75,11 +100,6 @@ export function Filters() { @@ -87,11 +107,9 @@ export function Filters() { {levelOptions.map((option) => (
diff --git a/features/ui/form/input/input.tsx b/features/ui/form/input/input.tsx index dad88cc..dc80304 100644 --- a/features/ui/form/input/input.tsx +++ b/features/ui/form/input/input.tsx @@ -29,7 +29,6 @@ const InputContainer = styled.input<{ border-color: ${({ errorMessage, error }) => errorMessage || error ? color("error", 300) : color("gray", 300)}; border-radius: 7px; - width: calc(${space(20)} * 4 - ${space(6)}); padding: ${space(2, 3)}; letter-spacing: 0.05rem; color: ${color("gray", 900)}; diff --git a/features/ui/form/select/select.stories.tsx b/features/ui/form/select/select.stories.tsx index 10ed054..4cc5ae4 100644 --- a/features/ui/form/select/select.stories.tsx +++ b/features/ui/form/select/select.stories.tsx @@ -41,7 +41,6 @@ Default.args = { label: "Team member", hint: "This is a hint text to help user.", errorMessage: "", - width: "", }; Default.parameters = { viewMode: "docs", diff --git a/features/ui/form/select/select.tsx b/features/ui/form/select/select.tsx index 8d5ca4c..b487eb2 100644 --- a/features/ui/form/select/select.tsx +++ b/features/ui/form/select/select.tsx @@ -1,9 +1,4 @@ -import React, { - useState, - ReactNode, - useRef, - SelectHTMLAttributes, -} from "react"; +import React, { useState, ReactNode, useRef, HTMLAttributes } from "react"; import styled, { css } from "styled-components"; import { useClickAway } from "react-use"; import { SelectContext } from "./selectContext"; @@ -14,14 +9,13 @@ export type OptionType = { value: string; }; -type SelectProps = Omit, "onChange"> & { +type SelectProps = Omit, "onChange"> & { children: ReactNode | ReactNode[]; errorMessage?: string; defaultValue?: string; placeholder?: string; disabled?: boolean; iconSrc?: string; - width?: string | number; label?: string; hint?: string; value?: string; @@ -29,16 +23,15 @@ type SelectProps = Omit, "onChange"> & { options: OptionType[]; }; -const Container = styled.div` +const Container = styled.div` position: relative; display: block; - width: ${({ width }) => width || `calc(${space(20)} * 4)`}; background-color: #fff; `; const List = styled.ul<{ showDropdown: boolean }>` display: block; - width: 100%; + min-width: 100%; margin: 0; padding: 0; position: absolute; @@ -63,15 +56,15 @@ const List = styled.ul<{ showDropdown: boolean }>` const SelectedOption = styled.div.attrs(() => ({ tabIndex: 0, ariaHasPopup: "listbox", -}))` +}))<{ disabled: boolean; hasError: boolean; isSelected: boolean }>` + width: 100%; border: 1px solid; - border-color: ${({ disabled, errorMessage }) => - !disabled && errorMessage ? color("error", 300) : color("gray", 300)}; + border-color: ${({ disabled, hasError }) => + !disabled && hasError ? color("error", 300) : color("gray", 300)}; border-radius: 7px; - width: ${({ width }) => width || `calc(${space(20)} * 4 - ${space(6)})`}; padding: ${space(2, 3)}; - color: ${({ selectedOption }) => - selectedOption ? color("gray", 900) : color("gray", 500)}; + color: ${({ isSelected }) => + isSelected ? color("gray", 900) : color("gray", 500)}; cursor: pointer; display: flex; justify-content: space-between; @@ -80,8 +73,8 @@ const SelectedOption = styled.div.attrs(() => ({ &:focus { outline: 3px solid; - outline-color: ${({ disabled, errorMessage }) => - !disabled && errorMessage ? color("error", 100) : color("primary", 200)}; + outline-color: ${({ disabled, hasError }) => + !disabled && hasError ? color("error", 100) : color("primary", 200)}; } ${({ disabled }) => @@ -142,7 +135,6 @@ export function Select({ label, hint, errorMessage, - width, children, options, onChange, @@ -173,16 +165,14 @@ export function Select({ changeSelectedValue: updateSelectedOption, }} > - + {label && } setShowDropdown(!showDropdown)} - selectedOption={selectedOption} + isSelected={!!selectedOption} disabled={disabled} - errorMessage={errorMessage} + hasError={!!errorMessage} aria-expanded={showDropdown} - width={width} > {iconSrc && } From 60c651580197dcd52a1dc7bbb6179686d09002ba Mon Sep 17 00:00:00 2001 From: Johannes Kettmann Date: Tue, 6 Dec 2022 12:27:01 +0100 Subject: [PATCH 8/8] Support key events in select input --- features/ui/form/select/option.tsx | 13 ++++++++++++- features/ui/form/select/select.tsx | 12 ++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/features/ui/form/select/option.tsx b/features/ui/form/select/option.tsx index 773e1c1..96ee44f 100644 --- a/features/ui/form/select/option.tsx +++ b/features/ui/form/select/option.tsx @@ -45,11 +45,22 @@ export function Option({ children, value }: OptionProps) { const { changeSelectedValue, selectedValue } = useSelectContext(); const isCurrentlySelected = selectedValue === value; + const onClick = () => { + changeSelectedValue(value); + }; + + const onKeyDown = (event: React.KeyboardEvent) => { + if (event.code === "Space") { + onClick(); + } + }; + return ( changeSelectedValue(value)} + onClick={onClick} + onKeyDown={onKeyDown} role="option" > {children} diff --git a/features/ui/form/select/select.tsx b/features/ui/form/select/select.tsx index b487eb2..94a4554 100644 --- a/features/ui/form/select/select.tsx +++ b/features/ui/form/select/select.tsx @@ -158,6 +158,16 @@ export function Select({ ? undefined : options.find((option) => option.value === value); + const toggleDropDown = () => { + setShowDropdown(!showDropdown); + }; + + const onKeyDown = (event: React.KeyboardEvent) => { + if (event.code === "Space") { + toggleDropDown(); + } + }; + return ( {label}}