-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: table "inline" editing #8754
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
base: main
Are you sure you want to change the base?
Changes from all commits
957f321
097ba50
98e8d84
7ea9be0
f447491
59bea14
0bff5e3
feb8a96
9c6e6f5
337b809
6be0394
a887e36
150efc7
a471192
7ef763a
bffae5a
7dd5a30
3b478af
65c4e61
9679410
2a80b8b
b02b164
b2e8d7e
b56a69a
308a5ec
e2dfc1c
41dbd4b
bc6c359
b9d2f2e
745afcb
1f641a4
ec72958
70a6336
08623b2
871bcda
f8ad4f7
bcb7c0e
b1e2303
bd79935
ad83b85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,21 +10,27 @@ | |
* governing permissions and limitations under the License. | ||
*/ | ||
|
||
import {ActionButton, ActionButtonContext} from './ActionButton'; | ||
import {baseColor, colorMix, focusRing, fontRelative, lightDark, space, style} from '../style' with {type: 'macro'}; | ||
import { | ||
Button, | ||
ButtonContext, | ||
CellRenderProps, | ||
Collection, | ||
ColumnRenderProps, | ||
ColumnResizer, | ||
ContextValue, | ||
DEFAULT_SLOT, | ||
Form, | ||
Key, | ||
OverlayTriggerStateContext, | ||
Provider, | ||
Cell as RACCell, | ||
CellProps as RACCellProps, | ||
CheckboxContext as RACCheckboxContext, | ||
Column as RACColumn, | ||
ColumnProps as RACColumnProps, | ||
Popover as RACPopover, | ||
Row as RACRow, | ||
RowProps as RACRowProps, | ||
Table as RACTable, | ||
|
@@ -44,9 +50,11 @@ import { | |
useTableOptions, | ||
Virtualizer | ||
} from 'react-aria-components'; | ||
import {centerPadding, controlFont, getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; | ||
import {centerPadding, colorScheme, controlFont, getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; | ||
import {Checkbox} from './Checkbox'; | ||
import Checkmark from '../s2wf-icons/S2_Icon_Checkmark_20_N.svg'; | ||
import Chevron from '../ui-icons/Chevron'; | ||
import Close from '../s2wf-icons/S2_Icon_Close_20_N.svg'; | ||
import {ColumnSize} from '@react-types/table'; | ||
import {DOMRef, DOMRefValue, forwardRefType, GlobalDOMAttributes, LoadingState, Node} from '@react-types/shared'; | ||
import {GridNode} from '@react-types/grid'; | ||
|
@@ -58,11 +66,12 @@ import {Menu, MenuItem, MenuSection, MenuTrigger} from './Menu'; | |
import Nubbin from '../ui-icons/S2_MoveHorizontalTableWidget.svg'; | ||
import {ProgressCircle} from './ProgressCircle'; | ||
import {raw} from '../style/style-macro' with {type: 'macro'}; | ||
import React, {createContext, forwardRef, ReactElement, ReactNode, useCallback, useContext, useMemo, useRef, useState} from 'react'; | ||
import React, {createContext, CSSProperties, ForwardedRef, forwardRef, ReactElement, ReactNode, RefObject, useCallback, useContext, useMemo, useRef, useState} from 'react'; | ||
import SortDownArrow from '../s2wf-icons/S2_Icon_SortDown_20_N.svg'; | ||
import SortUpArrow from '../s2wf-icons/S2_Icon_SortUp_20_N.svg'; | ||
import {useActionBarContainer} from './ActionBar'; | ||
import {useDOMRef} from '@react-spectrum/utils'; | ||
import {useLayoutEffect, useObjectRef} from '@react-aria/utils'; | ||
import {useLocalizedStringFormatter} from '@react-aria/i18n'; | ||
import {useScale} from './utils'; | ||
import {useSpectrumContextProps} from './useSpectrumContextProps'; | ||
|
@@ -1044,6 +1053,193 @@ export const Cell = forwardRef(function Cell(props: CellProps, ref: DOMRef<HTMLD | |
); | ||
}); | ||
|
||
let editPopover = style({ | ||
...colorScheme(), | ||
'--s2-container-bg': { | ||
type: 'backgroundColor', | ||
value: 'layer-2' | ||
}, | ||
backgroundColor: '--s2-container-bg', | ||
borderBottomRadius: 'default', | ||
// Use box-shadow instead of filter when an arrow is not shown. | ||
// This fixes the shadow stacking problem with submenus. | ||
boxShadow: 'elevated', | ||
borderStyle: 'solid', | ||
borderWidth: 1, | ||
borderColor: { | ||
default: 'gray-200', | ||
forcedColors: 'ButtonBorder' | ||
}, | ||
boxSizing: 'content-box', | ||
isolation: 'isolate', | ||
pointerEvents: { | ||
isExiting: 'none' | ||
}, | ||
outlineStyle: 'none', | ||
minWidth: '--trigger-width', | ||
padding: 8, | ||
display: 'flex', | ||
alignItems: 'center' | ||
}, getAllowedOverrides()); | ||
|
||
interface EditableCellProps extends Omit<CellProps, 'isSticky'> { | ||
renderEditing: () => ReactNode, | ||
isSaving?: boolean, | ||
onSubmit: () => void, | ||
onCancel: () => void | ||
} | ||
|
||
/** | ||
* An exditable cell within a table row. | ||
*/ | ||
export const EditableCell = forwardRef(function EditableCell(props: EditableCellProps, ref: ForwardedRef<HTMLDivElement>) { | ||
let {children, showDivider = false, textValue, ...otherProps} = props; | ||
let tableVisualOptions = useContext(InternalTableContext); | ||
let domRef = useObjectRef(ref); | ||
textValue ||= typeof children === 'string' ? children : undefined; | ||
|
||
return ( | ||
<RACCell | ||
ref={domRef} | ||
className={renderProps => cell({ | ||
...renderProps, | ||
...tableVisualOptions, | ||
isDivider: showDivider | ||
})} | ||
textValue={textValue} | ||
{...otherProps}> | ||
{({isFocusVisible}) => ( | ||
<EditableCellInner {...props} isFocusVisible={isFocusVisible} cellRef={domRef as RefObject<HTMLDivElement>} /> | ||
)} | ||
</RACCell> | ||
); | ||
}); | ||
|
||
function EditableCellInner(props: EditableCellProps & {isFocusVisible: boolean, cellRef: RefObject<HTMLDivElement>}) { | ||
let {children, align, renderEditing, isSaving, onSubmit, onCancel, isFocusVisible, cellRef} = props; | ||
let [isOpen, setIsOpen] = useState(false); | ||
let popoverRef = useRef<HTMLDivElement>(null); | ||
let formRef = useRef<HTMLFormElement>(null); | ||
let [triggerWidth, setTriggerWidth] = useState(0); | ||
let [tableWidth, setTableWidth] = useState(0); | ||
let [verticalOffset, setVerticalOffset] = useState(0); | ||
let tableVisualOptions = useContext(InternalTableContext); | ||
let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/s2'); | ||
|
||
let {density} = useContext(InternalTableContext); | ||
let size: 'XS' | 'S' | 'M' | 'L' | 'XL' | undefined = 'M'; | ||
if (density === 'compact') { | ||
size = 'S'; | ||
} else if (density === 'spacious') { | ||
size = 'L'; | ||
} | ||
|
||
|
||
// Popover positioning | ||
useLayoutEffect(() => { | ||
if (!isOpen) { | ||
return; | ||
} | ||
let width = cellRef.current?.clientWidth || 0; | ||
let cell = cellRef.current; | ||
let boundingRect = cell?.parentElement?.getBoundingClientRect(); | ||
let verticalOffset = (boundingRect?.top ?? 0) - (boundingRect?.bottom ?? 0); | ||
|
||
let tableWidth = cellRef.current?.closest('[role="grid"]')?.clientWidth || 0; | ||
setTriggerWidth(width); | ||
setVerticalOffset(verticalOffset); | ||
setTableWidth(tableWidth); | ||
}, [cellRef, density, isOpen]); | ||
|
||
// Cancel, don't save the value | ||
let cancel = () => { | ||
setIsOpen(false); | ||
onCancel(); | ||
}; | ||
|
||
return ( | ||
<Provider | ||
values={[ | ||
[ButtonContext, null], | ||
[ActionButtonContext, { | ||
slots: { | ||
[DEFAULT_SLOT]: {}, | ||
edit: { | ||
onPress: () => setIsOpen(true), | ||
isPending: isSaving, | ||
isQuiet: !isSaving, | ||
size, | ||
excludeFromTabOrder: true, | ||
styles: style({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably ok? probably not common that people keyboard focus, then scroll without clicking somewhere There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bit unfortunate that the button is sized/the row outlines are made in such a way that the top of the ring sits flush under the top row's border but the bottom of the ring sits on top of the border. Not too bothered by it, we can look into it as a followup |
||
// TODO: really need access to display here instead, but not possible right now | ||
// will be addressable with displayOuter | ||
visibility: { | ||
default: 'hidden', | ||
isForcedVisible: 'visible', | ||
':is([role="row"]:hover *)': 'visible', | ||
':is([role="row"][data-focus-visible-within] *)': 'visible', | ||
'@media not (any-pointer: fine)': 'visible' | ||
} | ||
})({isForcedVisible: isOpen || !!isSaving}) | ||
} | ||
} | ||
}] | ||
]}> | ||
<span className={cellContent({...tableVisualOptions, align: align || 'start'})}>{children}</span> | ||
{isFocusVisible && <CellFocusRing />} | ||
|
||
<Provider | ||
values={[ | ||
[ActionButtonContext, null] | ||
]}> | ||
<RACPopover | ||
isOpen={isOpen} | ||
onOpenChange={setIsOpen} | ||
ref={popoverRef} | ||
shouldCloseOnInteractOutside={() => { | ||
if (!popoverRef.current?.contains(document.activeElement)) { | ||
return false; | ||
} | ||
formRef.current?.requestSubmit(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might just be forgetting prior examples, but I feel like dismissing a popover via clicking outside shouldn't be a confirm/submit action but instead just a cancel action There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Design specifically says this should confirm 🤷🏻 |
||
return false; | ||
}} | ||
triggerRef={cellRef} | ||
aria-label={stringFormatter.format('table.editCell')} | ||
offset={verticalOffset} | ||
placement="bottom start" | ||
style={{ | ||
minWidth: `min(${triggerWidth}px, ${tableWidth}px)`, | ||
maxWidth: `${tableWidth}px`, | ||
// Override default z-index from useOverlayPosition. We use isolation: isolate instead. | ||
zIndex: undefined | ||
}} | ||
className={editPopover}> | ||
<Provider | ||
values={[ | ||
[OverlayTriggerStateContext, null] | ||
]}> | ||
<Form | ||
ref={formRef} | ||
onSubmit={(e) => { | ||
e.preventDefault(); | ||
onSubmit(); | ||
setIsOpen(false); | ||
}} | ||
className={style({width: 'full', display: 'flex', alignItems: 'baseline', gap: 16})} | ||
style={{'--input-width': `calc(${triggerWidth}px - 32px)`} as CSSProperties}> | ||
{renderEditing()} | ||
<div className={style({display: 'flex', flexDirection: 'row', alignItems: 'baseline', flexShrink: 0, flexGrow: 0})}> | ||
<ActionButton isQuiet onPress={cancel} aria-label={stringFormatter.format('table.cancel')}><Close /></ActionButton> | ||
<ActionButton isQuiet type="submit" aria-label={stringFormatter.format('table.save')}><Checkmark /></ActionButton> | ||
</div> | ||
</Form> | ||
</Provider> | ||
</RACPopover> | ||
</Provider> | ||
</Provider> | ||
); | ||
} | ||
|
||
// Use color-mix instead of transparency so sticky cells work correctly. | ||
const selectedBackground = lightDark(colorMix('gray-25', 'informative-900', 10), colorMix('gray-25', 'informative-700', 10)); | ||
const selectedActiveBackground = lightDark(colorMix('gray-25', 'informative-900', 15), colorMix('gray-25', 'informative-700', 15)); | ||
|
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.
one thing I noticed when testing is that tabbing through the table will move you to the edit buttons instead of treating the table as a single tab stop like it does for v3 table, not quite sure why. Note that this specifically happens when selection is enabled: https://reactspectrum.blob.core.windows.net/reactspectrum/f8ad4f7b2000787617b3b6097af722ecf8b5679f/storybook-s2/index.html?path=/story/tableview--editable-table&args=selectionMode:multiple
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.
hmmm odd, i'll have a look
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.
o, i see
lastChild is the last checkbox, which is in a row, we focus it without scrolling, which makes the edit button visible
then we let tab naturally move us to the next element, which is the now visible edit button...
I think i've fixed it by applying excludeFromTabOrder since the collection should handle moving to and from it
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.
added tests as well