Skip to content

Commit

Permalink
fix: Cell actions should have correct background when row focused wit…
Browse files Browse the repository at this point in the history
…hin (#25790)

* fix: Cell actions should have correct background when row focused within

The table cell actions should appear when any focus is within the row
and not just within the cell actions container. Also apply the hover
background colour when the row has focus within.

Now when the cell actions overlap the cell content, it will 'cover' the
content just like in hover scenarios.

* changefiles
  • Loading branch information
ling1726 authored Dec 1, 2022
1 parent 6ad0aee commit 6f450a1
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "fix: Cell actions should have correct background when row focused within",
"packageName": "@fluentui/react-table",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: focus-within attribute no longer the same as focus-visible",
"packageName": "@fluentui/react-tabster",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as React from 'react';
import { getNativeElementProps, useMergedRefs } from '@fluentui/react-utilities';
import { useFocusWithin } from '@fluentui/react-tabster';
import { getNativeElementProps } from '@fluentui/react-utilities';
import type { TableCellActionsProps, TableCellActionsState } from './TableCellActions.types';

/**
Expand All @@ -21,7 +20,7 @@ export const useTableCellActions_unstable = (
root: 'div',
},
root: getNativeElementProps('div', {
ref: useMergedRefs(ref, useFocusWithin()),
ref,
...props,
}),
visible: props.visible ?? false,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { makeStyles, mergeClasses } from '@griffel/react';
import type { TableCellActionsSlots, TableCellActionsState } from './TableCellActions.types';
import type { SlotClassNames } from '@fluentui/react-utilities';
import { createCustomFocusIndicatorStyle } from '@fluentui/react-tabster';

export const tableCellActionsClassNames: SlotClassNames<TableCellActionsSlots> = {
root: 'fui-TableCellActions',
Expand All @@ -18,13 +17,6 @@ const useStyles = makeStyles({
transform: 'translateY(-50%)',
opacity: 0,
marginLeft: 'auto',

...createCustomFocusIndicatorStyle(
{
opacity: 1,
},
{ selector: 'focus-within' },
),
},

visible: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react';
import { getNativeElementProps, useMergedRefs } from '@fluentui/react-utilities';
import type { TableRowProps, TableRowState } from './TableRow.types';
import { useTableContext } from '../../contexts/tableContext';
import { useFocusVisible } from '@fluentui/react-tabster';
import { useFocusVisible, useFocusWithin } from '@fluentui/react-tabster';

/**
* Create the state required to render TableRow.
Expand All @@ -17,13 +17,14 @@ export const useTableRow_unstable = (props: TableRowProps, ref: React.Ref<HTMLEl
const { noNativeElements, size } = useTableContext();
const rootComponent = props.as ?? noNativeElements ? 'div' : 'tr';
const focusVisibleRef = useFocusVisible();
const focusWithinRef = useFocusWithin();

return {
components: {
root: rootComponent,
},
root: getNativeElementProps(rootComponent, {
ref: useMergedRefs(ref, focusVisibleRef),
ref: useMergedRefs(ref, focusVisibleRef, focusWithinRef),
role: rootComponent === 'div' ? 'row' : undefined,
...props,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ const useStyles = makeStyles({
[`& .${tableSelectionCellClassNames.root}`]: {
opacity: 1,
},
[`& .${tableCellActionsClassNames.root}`]: {
opacity: 1,
},
},
{ selector: 'focus-within' },
),
Expand All @@ -49,6 +52,22 @@ const useStyles = makeStyles({
),
},

// When focus is within the row the background colour
// should be the same as hover, except when there is a brand
// or neutral appearance applied on the row
noAppearanceFocusWithin: {
...createCustomFocusIndicatorStyle(
{
[`& .${tableCellActionsClassNames.root}`]: {
backgroundColor: tokens.colorSubtleBackgroundHover,
},

backgroundColor: tokens.colorSubtleBackgroundHover,
},
{ selector: 'focus-within' },
),
},

rootInteractive: {
':active': {
backgroundColor: tokens.colorSubtleBackgroundPressed,
Expand Down Expand Up @@ -145,6 +164,7 @@ export const useTableRowStyles_unstable = (state: TableRowState): TableRowState
styles[state.size],
state.noNativeElements ? layoutStyles.flex.root : layoutStyles.table.root,
styles[state.appearance],
state.appearance === 'none' && !isHeaderRow && styles.noAppearanceFocusWithin,
state.root.className,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const FOCUS_VISIBLE_ATTR = 'data-fui-focus-visible';
/**
* @internal
*/
export const FOCUS_WITHIN_ATTR = 'data-fui-focus-visible';
export const FOCUS_WITHIN_ATTR = 'data-fui-focus-within';
export const defaultOptions = {
style: {},
selector: 'focus',
Expand Down

0 comments on commit 6f450a1

Please sign in to comment.