-
Notifications
You must be signed in to change notification settings - Fork 0
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
Enable shopping list title to be edited from shopping lists page #40
Merged
danascheider
merged 15 commits into
shopping-lists-page-feature-branch
from
265-enable-shopping-list-title-to-be-edited-from-shopping-lists-page
Mar 29, 2023
Merged
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
18965cb
Add @fortawesome/free-regular-svg-icons package
danascheider 6920158
Update fontawesome icons and update snapshots
danascheider 1384c56
Update Node to 19.8.1
danascheider b025c3a
Basic list edit form component to go in shopping/inventory list headers
danascheider 4315336
Specify node version
danascheider 93c71ab
Add useComponentVisible and useSize hooks for use in ShoppingList com…
danascheider a6096ed
Incorporate shopping list edit form into ShoppingList component (not …
danascheider e6f76b0
Add function for PATCHing shopping lists to API wrapper
danascheider b926749
Wire up list edit form to actually edit lists
danascheider 2754d1a
Fix issues with spacing on the list edit form
danascheider 2041571
Add tests for error cases
danascheider 74c2084
Update docs
danascheider aeaeddb
Skip test that would require node-canvas to support worker threads
danascheider 1123def
Remove unused import
danascheider e10c6ae
Make changes suggested by code review
danascheider File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
yarn 1.22.19 | ||
nodejs 19.6.1 | ||
nodejs 19.8.1 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
.root { | ||
display: grid; | ||
grid-template-columns: auto min-content; | ||
max-width: var(--max-width); | ||
background-color: var(--scheme-color); | ||
align-content: center; | ||
border-bottom: 1px solid var(--border-color); | ||
} | ||
|
||
.input { | ||
font-family: 'Quattrocento Sans', Arial, Helvetica, sans-serif; | ||
font-size: 1.25rem; | ||
font-weight: 700; | ||
padding: 0; | ||
padding-right: 8px; | ||
background-color: inherit; | ||
color: var(--text-color); | ||
border-style: none; | ||
} | ||
|
||
.submit { | ||
background-color: inherit; | ||
color: var(--text-color); | ||
cursor: pointer; | ||
flex: 0; | ||
padding-right: 0; | ||
border-style: none; | ||
} | ||
|
||
.fa { | ||
color: var(--text-color); | ||
font-size: 17px; | ||
margin-bottom: -2px; | ||
} | ||
|
||
.fa:hover { | ||
color: var(--icon-hover-color); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { useRef, type FormEvent } from 'react' | ||
import { PINK } from '../../utils/colorSchemes' | ||
import { ColorProvider } from '../../contexts/colorContext' | ||
import ListEditForm from './listEditForm' | ||
|
||
const WrapperComponent = () => { | ||
const formRef = useRef<HTMLFormElement>(null) | ||
|
||
return ( | ||
<div> | ||
<ListEditForm | ||
className="foo" | ||
formRef={formRef} | ||
maxTotalWidth={256} | ||
title="Severin Manor" | ||
onSubmit={(e: FormEvent) => e.preventDefault()} | ||
/> | ||
</div> | ||
) | ||
} | ||
|
||
export default { title: 'ListEditForm' } | ||
|
||
export const Default = () => ( | ||
<ColorProvider colorScheme={PINK}> | ||
<WrapperComponent /> | ||
</ColorProvider> | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import { useRef, type FormEvent, type FormEventHandler } from 'react' | ||
import { describe, test, expect, vitest } from 'vitest' | ||
import { act, fireEvent } from '@testing-library/react' | ||
import { render } from '../../support/testUtils' | ||
import { BLUE } from '../../utils/colorSchemes' | ||
import { ColorProvider } from '../../contexts/colorContext' | ||
import ListEditForm from './listEditForm' | ||
|
||
interface WrapperProps { | ||
onSubmit: FormEventHandler | ||
} | ||
|
||
const WrapperComponent = ({ onSubmit }: WrapperProps) => { | ||
const formRef = useRef(null) | ||
|
||
return ( | ||
<div> | ||
<ListEditForm | ||
className="foo" | ||
formRef={formRef} | ||
maxTotalWidth={256} | ||
title="Alchemy Ingredients" | ||
onSubmit={onSubmit} | ||
/> | ||
</div> | ||
) | ||
} | ||
|
||
describe('ListEditForm', () => { | ||
test.skip('displays the title and submit button', () => { | ||
const wrapper = render( | ||
<ColorProvider colorScheme={BLUE}> | ||
<WrapperComponent onSubmit={(e: FormEvent) => e.preventDefault()} /> | ||
</ColorProvider> | ||
) | ||
|
||
expect(wrapper).toBeTruthy() | ||
expect( | ||
wrapper.getByRole('textbox').attributes.getNamedItem('name') | ||
).toEqual('title') | ||
expect(wrapper.getByRole('button').attributes.getNamedItem('name')).toEqual( | ||
'submit' | ||
) | ||
}) | ||
|
||
describe('submitting the form', () => { | ||
test('calls the onSubmit function passed in when button is clicked', () => { | ||
const onSubmit = vitest.fn() | ||
const wrapper = render( | ||
<ColorProvider colorScheme={BLUE}> | ||
<WrapperComponent onSubmit={onSubmit} /> | ||
</ColorProvider> | ||
) | ||
|
||
const input = wrapper.getByRole('textbox') | ||
const button = wrapper.getByRole('button') | ||
|
||
act(() => { | ||
fireEvent.change(input, { target: { value: 'Smithing Materials' } }) | ||
fireEvent.click(button) | ||
}) | ||
|
||
expect(onSubmit).toHaveBeenCalledOnce() | ||
}) | ||
|
||
test('calls the onSubmit function any time the form is submitted', () => { | ||
const onSubmit = vitest.fn() | ||
const wrapper = render( | ||
<ColorProvider colorScheme={BLUE}> | ||
<WrapperComponent onSubmit={onSubmit} /> | ||
</ColorProvider> | ||
) | ||
|
||
const input = wrapper.getByRole('textbox') | ||
const form = wrapper.getByRole('form') | ||
|
||
act(() => { | ||
fireEvent.change(input, { target: { value: 'Smithing Materials' } }) | ||
fireEvent.submit(form) | ||
}) | ||
|
||
expect(onSubmit).toHaveBeenCalledOnce() | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
import { | ||
useState, | ||
useRef, | ||
useEffect, | ||
type FormEventHandler, | ||
type CSSProperties, | ||
type ChangeEvent, | ||
type RefObject, | ||
} from 'react' | ||
import classNames from 'classnames' | ||
import { useColorScheme } from '../../hooks/contexts' | ||
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' | ||
import { faSquareCheck } from '@fortawesome/free-regular-svg-icons' | ||
import styles from './listEditForm.module.css' | ||
|
||
const FIXED_BUTTON_WIDTH = 72 // px | ||
|
||
interface EditFormProps { | ||
formRef: RefObject<any> | ||
maxTotalWidth: number | ||
className?: string | ||
title: string | ||
onSubmit: FormEventHandler | ||
} | ||
|
||
const ListEditForm = ({ | ||
formRef, | ||
maxTotalWidth, | ||
className, | ||
title, | ||
onSubmit, | ||
}: EditFormProps) => { | ||
const [maxTextWidth, setMaxTextWidth] = useState<number>( | ||
danascheider marked this conversation as resolved.
Show resolved
Hide resolved
|
||
maxTotalWidth - FIXED_BUTTON_WIDTH - 2 | ||
) | ||
|
||
const getInputTextWidth = (text: string) => { | ||
const canvas = document.createElement('canvas') | ||
const context = canvas.getContext('2d') | ||
|
||
if (!context) return 0 | ||
|
||
context.font = '21px Quattrocento Sans' | ||
|
||
const textWidth = context.measureText(text).width | ||
|
||
return Math.min(textWidth, maxTextWidth) | ||
} | ||
|
||
const [inputValue, setInputValue] = useState(title) | ||
const [inputWidth, setInputWidth] = useState( | ||
`${getInputTextWidth(inputValue)}px` | ||
) | ||
|
||
const inputRef = useRef<HTMLInputElement>(null) | ||
const buttonRef = useRef<HTMLButtonElement>(null) | ||
|
||
const { | ||
schemeColorDarkest, | ||
textColorPrimary, | ||
borderColor, | ||
schemeColorLightest, | ||
} = useColorScheme() | ||
|
||
const rootStyles = { | ||
'--scheme-color': schemeColorDarkest, | ||
'--text-color': textColorPrimary, | ||
'--border-color': borderColor, | ||
'--icon-hover-color': schemeColorLightest, | ||
'--max-width': `${maxTotalWidth}px`, | ||
} as CSSProperties | ||
|
||
const updateInputWidth = (e: ChangeEvent) => { | ||
const newValue = (e.currentTarget as HTMLInputElement)?.value || '' | ||
setInputValue(newValue) | ||
setInputWidth(`${getInputTextWidth(newValue)}px`) | ||
} | ||
|
||
useEffect(() => { | ||
inputRef.current?.focus() | ||
}, []) | ||
|
||
useEffect(() => { | ||
setMaxTextWidth(maxTotalWidth - FIXED_BUTTON_WIDTH - 2) | ||
setInputWidth(`${getInputTextWidth(inputValue)}px`) | ||
}, [maxTotalWidth]) | ||
|
||
return ( | ||
<form | ||
className={classNames(className, styles.root)} | ||
style={rootStyles} | ||
ref={formRef} | ||
onSubmit={onSubmit} | ||
aria-label="List title edit form" | ||
> | ||
<input | ||
className={styles.input} | ||
onClick={(e) => e.stopPropagation()} | ||
onChange={updateInputWidth} | ||
type="text" | ||
name="title" | ||
aria-label="title" | ||
ref={inputRef} | ||
style={{ width: inputWidth }} | ||
defaultValue={inputValue} | ||
pattern="^\s*[A-Za-z0-9 \-',]*\s*$" | ||
title="Title can only contain alphanumeric characters, spaces, hyphens, commas, and apostrophes" | ||
data-testid="editListTitle" | ||
/> | ||
<button | ||
className={styles.submit} | ||
ref={buttonRef} | ||
name="submit" | ||
type="submit" | ||
> | ||
<FontAwesomeIcon className={styles.fa} icon={faSquareCheck} /> | ||
</button> | ||
</form> | ||
) | ||
} | ||
|
||
export default ListEditForm |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is being skipped because rendering the form in JSDOM involves using the node-canvas package, which doesn't support worker_threads. What are
worker_threads
and are we using them in this application? Apparently we are. The maintainers have said they would like to add support this year, so I'm skipping this test rather than deleting it entirely.