From 52f526a76e21ad02f8569529149806d42f52b893 Mon Sep 17 00:00:00 2001 From: joshwooding <12938082+joshwooding@users.noreply.github.com> Date: Thu, 7 May 2020 00:20:56 +0100 Subject: [PATCH 1/4] Add more comments --- packages/material-ui-lab/src/TreeItem/TreeItem.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.js b/packages/material-ui-lab/src/TreeItem/TreeItem.js index 149db4c81ede70..a9830857cac4ae 100644 --- a/packages/material-ui-lab/src/TreeItem/TreeItem.js +++ b/packages/material-ui-lab/src/TreeItem/TreeItem.js @@ -189,6 +189,7 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) { const handleMouseDown = (event) => { if (event.shiftKey || event.ctrlKey || event.metaKey) { + // Prevent text selection event.preventDefault(); } @@ -363,7 +364,12 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) { if (multiSelect) { ariaSelected = selected; } else if (selected) { - // single-selection trees unset aria-selected + /* single-selection trees unset aria-selected on un-selected items. + * + * If the tree does not support multiple selection, aria-selected + * is set to true for the selected node and it is not present on any other node in the tree. + * Source: https://www.w3.org/TR/wai-aria-practices/#TreeView + */ ariaSelected = true; } From a4647f29c30e3f4384f96dd1a6fb822d1743b632 Mon Sep 17 00:00:00 2001 From: joshwooding <12938082+joshwooding@users.noreply.github.com> Date: Thu, 7 May 2020 00:32:38 +0100 Subject: [PATCH 2/4] Experiment with simplifying styling --- .../components/tree-view/GmailTreeView.js | 23 ++++--- .../components/tree-view/GmailTreeView.tsx | 23 ++++--- .../src/TreeItem/TreeItem.d.ts | 1 + .../material-ui-lab/src/TreeItem/TreeItem.js | 63 ++++++++++--------- .../src/TreeItem/TreeItem.test.js | 1 - 5 files changed, 56 insertions(+), 55 deletions(-) diff --git a/docs/src/pages/components/tree-view/GmailTreeView.js b/docs/src/pages/components/tree-view/GmailTreeView.js index c789a46d516762..1d7a0d29d0582d 100644 --- a/docs/src/pages/components/tree-view/GmailTreeView.js +++ b/docs/src/pages/components/tree-view/GmailTreeView.js @@ -17,16 +17,6 @@ import ArrowRightIcon from '@material-ui/icons/ArrowRight'; const useTreeItemStyles = makeStyles((theme) => ({ root: { color: theme.palette.text.secondary, - '&:hover > $content': { - backgroundColor: theme.palette.action.hover, - }, - '&:focus > $content, &$selected > $content': { - backgroundColor: `var(--tree-view-bg-color, ${theme.palette.grey[400]})`, - color: 'var(--tree-view-color)', - }, - '&:focus > $content $label, &:hover > $content $label, &$selected > $content $label': { - backgroundColor: 'transparent', - }, }, content: { color: theme.palette.text.secondary, @@ -34,9 +24,16 @@ const useTreeItemStyles = makeStyles((theme) => ({ borderBottomRightRadius: theme.spacing(2), paddingRight: theme.spacing(1), fontWeight: theme.typography.fontWeightMedium, - '$expanded > &': { + '&$expanded': { fontWeight: theme.typography.fontWeightRegular, }, + '&:hover': { + backgroundColor: theme.palette.action.hover, + }, + '&$focused, &$selected': { + backgroundColor: `var(--tree-view-bg-color, ${theme.palette.grey[400]})`, + color: 'var(--tree-view-color)', + }, }, group: { marginLeft: 0, @@ -46,6 +43,7 @@ const useTreeItemStyles = makeStyles((theme) => ({ }, expanded: {}, selected: {}, + focused: {}, label: { fontWeight: 'inherit', color: 'inherit', @@ -53,7 +51,7 @@ const useTreeItemStyles = makeStyles((theme) => ({ labelRoot: { display: 'flex', alignItems: 'center', - padding: theme.spacing(0.5, 0), + padding: theme.spacing(0.5, 0, 0.5, 0.5), }, labelIcon: { marginRight: theme.spacing(1), @@ -97,6 +95,7 @@ function StyledTreeItem(props) { content: classes.content, expanded: classes.expanded, selected: classes.selected, + focused: classes.focused, group: classes.group, label: classes.label, }} diff --git a/docs/src/pages/components/tree-view/GmailTreeView.tsx b/docs/src/pages/components/tree-view/GmailTreeView.tsx index 626e386080b432..cf690ce2380ef5 100644 --- a/docs/src/pages/components/tree-view/GmailTreeView.tsx +++ b/docs/src/pages/components/tree-view/GmailTreeView.tsx @@ -33,16 +33,6 @@ const useTreeItemStyles = makeStyles((theme: Theme) => createStyles({ root: { color: theme.palette.text.secondary, - '&:hover > $content': { - backgroundColor: theme.palette.action.hover, - }, - '&:focus > $content, &$selected > $content': { - backgroundColor: `var(--tree-view-bg-color, ${theme.palette.grey[400]})`, - color: 'var(--tree-view-color)', - }, - '&:focus > $content $label, &:hover > $content $label, &$selected > $content $label': { - backgroundColor: 'transparent', - }, }, content: { color: theme.palette.text.secondary, @@ -50,9 +40,16 @@ const useTreeItemStyles = makeStyles((theme: Theme) => borderBottomRightRadius: theme.spacing(2), paddingRight: theme.spacing(1), fontWeight: theme.typography.fontWeightMedium, - '$expanded > &': { + '&$expanded': { fontWeight: theme.typography.fontWeightRegular, }, + '&:hover': { + backgroundColor: theme.palette.action.hover, + }, + '&$focused, &$selected': { + backgroundColor: `var(--tree-view-bg-color, ${theme.palette.grey[400]})`, + color: 'var(--tree-view-color)', + }, }, group: { marginLeft: 0, @@ -62,6 +59,7 @@ const useTreeItemStyles = makeStyles((theme: Theme) => }, expanded: {}, selected: {}, + focused: {}, label: { fontWeight: 'inherit', color: 'inherit', @@ -69,7 +67,7 @@ const useTreeItemStyles = makeStyles((theme: Theme) => labelRoot: { display: 'flex', alignItems: 'center', - padding: theme.spacing(0.5, 0), + padding: theme.spacing(0.5, 0, 0.5, 0.5), }, labelIcon: { marginRight: theme.spacing(1), @@ -114,6 +112,7 @@ function StyledTreeItem(props: StyledTreeItemProps) { content: classes.content, expanded: classes.expanded, selected: classes.selected, + focused: classes.focused, group: classes.group, label: classes.label, }} diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.d.ts b/packages/material-ui-lab/src/TreeItem/TreeItem.d.ts index aff614ab7ddbbb..5d57784f47d147 100644 --- a/packages/material-ui-lab/src/TreeItem/TreeItem.d.ts +++ b/packages/material-ui-lab/src/TreeItem/TreeItem.d.ts @@ -55,6 +55,7 @@ export type TreeItemClassKey = | 'root' | 'expanded' | 'selected' + | 'focused' | 'group' | 'content' | 'iconContainer' diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.js b/packages/material-ui-lab/src/TreeItem/TreeItem.js index a9830857cac4ae..f166a942bbd4a1 100644 --- a/packages/material-ui-lab/src/TreeItem/TreeItem.js +++ b/packages/material-ui-lab/src/TreeItem/TreeItem.js @@ -15,14 +15,34 @@ export const styles = (theme) => ({ margin: 0, padding: 0, outline: 0, + }, + /* Styles applied to the `role="group"` element. */ + group: { + margin: 0, + padding: 0, + marginLeft: 17, + }, + /* Styles applied to the tree node content. */ + content: { + width: '100%', + display: 'flex', + alignItems: 'center', + cursor: 'pointer', WebkitTapHighlightColor: 'transparent', - '&:focus > $content $label': { + '&:hover': { + backgroundColor: theme.palette.action.hover, + // Reset on touch devices, it doesn't add specificity + '@media (hover: none)': { + backgroundColor: 'transparent', + }, + }, + '&$focused': { backgroundColor: theme.palette.action.hover, }, - '&$selected > $content $label': { + '&$selected': { backgroundColor: fade(theme.palette.primary.main, theme.palette.action.selectedOpacity), }, - '&$selected > $content $label:hover, &$selected:focus > $content $label': { + '&$selected:hover, &$selected$focused': { backgroundColor: fade( theme.palette.primary.main, theme.palette.action.selectedOpacity + theme.palette.action.hoverOpacity, @@ -33,23 +53,12 @@ export const styles = (theme) => ({ }, }, }, - /* Pseudo-class applied to the root element when expanded. */ + /* Pseudo-class applied to the content element when expanded. */ expanded: {}, - /* Pseudo-class applied to the root element when selected. */ + /* Pseudo-class applied to the content element when selected. */ selected: {}, - /* Styles applied to the `role="group"` element. */ - group: { - margin: 0, - padding: 0, - marginLeft: 17, - }, - /* Styles applied to the tree node content. */ - content: { - width: '100%', - display: 'flex', - alignItems: 'center', - cursor: 'pointer', - }, + /* Pseudo-class applied to the content element when focused. */ + focused: {}, /* Styles applied to the tree node icon and collapse/expand icon. */ iconContainer: { marginRight: 4, @@ -66,13 +75,6 @@ export const styles = (theme) => ({ width: '100%', paddingLeft: 4, position: 'relative', - '&:hover': { - backgroundColor: theme.palette.action.hover, - // Reset on touch devices, it doesn't add specificity - '@media (hover: none)': { - backgroundColor: 'transparent', - }, - }, }, }); @@ -375,10 +377,7 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) { return (