Add editing #16
Add editing #16
Conversation
mattrothenberg
left a comment
mattrothenberg
left a comment
There was a problem hiding this comment.
Some open questions and a11y nitpicks but this looks solid!
Did you consider using an OSS component for the editable input pattern like the Chakra one below? Curious if you found anything cool
src/components/editable-cell.tsx
Outdated
| value={editedValue} | ||
| onChange={e => setEditedValue(e.target.value)} | ||
| onKeyDown={e => { | ||
| if (e.key === 'Enter') { |
There was a problem hiding this comment.
Nice! Did you try wrapping this input in form tag? We'd get this enter key functionality for free.
I noticed that https://chakra-ui.com/docs/form/editable does this the same way sans form tag but I'm curious.
There was a problem hiding this comment.
absolutely, good call! I shied away from it to cut down on DOM elements, but in retrospect it's only rendered for actively edited cells so that's silly
src/components/editable-header.tsx
Outdated
| css={[ | ||
| tw`h-full w-full max-w-full flex items-center cursor-cell`, | ||
| ]} | ||
| onClick={() => setIsEditing(true)} |
There was a problem hiding this comment.
Let's make this a button instead of a div (or add the aria attributes that provide the correct hinting that this is interactive).
src/components/editable-header.tsx
Outdated
| css={[ | ||
| tw`w-full h-full py-2 px-2 font-mono text-black text-sm focus:outline-none bg-transparent bg-white`, | ||
| ]} | ||
| style={{ fontSize: "0.875rem" }} |
There was a problem hiding this comment.
Is this inline style necessary? You have text-sm above too
There was a problem hiding this comment.
it was being overridden by a style in the stylesheet - I'll rejigger that, good call
src/components/editable-header.tsx
Outdated
| e.target.select(); | ||
| }} | ||
| css={[ | ||
| tw`w-full h-full py-2 px-2 font-mono text-black text-sm focus:outline-none bg-transparent bg-white`, |
There was a problem hiding this comment.
bg-transparent and bg-white?
src/components/header.tsx
Outdated
| tw="relative border-b border-gray-200 bg-white flex items-center flex-shrink-0" | ||
| style={{ height: 37 }} | ||
| // ref={popoverAnchorRef} | ||
| // ref={popoverAnchorRef} |
|
wonderful, thanks for the feedback @mattrothenberg! |
|
🚢 |
This adds the option to make the grid editable, similar to a spreadsheet.