From 56a74304ed3e78ec482dbe632dcf51a2c6977b1d Mon Sep 17 00:00:00 2001 From: Tom Brunet Date: Fri, 7 May 2021 09:35:10 -0500 Subject: [PATCH 01/16] ARIA fixes for OverflowMenuItem (#8599) OverflowMenuItem using a button inside of a menuitem rather than overriding the button to be the menuitem. --- .../react/src/components/OverflowMenuItem/OverflowMenuItem.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/OverflowMenuItem/OverflowMenuItem.js b/packages/react/src/components/OverflowMenuItem/OverflowMenuItem.js index 164bea218032..392b3448a323 100644 --- a/packages/react/src/components/OverflowMenuItem/OverflowMenuItem.js +++ b/packages/react/src/components/OverflowMenuItem/OverflowMenuItem.js @@ -186,12 +186,13 @@ export default class OverflowMenuItem extends React.Component { ); })(); return ( -
  • +
  • Date: Fri, 7 May 2021 08:24:11 -0700 Subject: [PATCH 02/16] fix(button): ignore fill="none" paths on icon svgs (#8585) * fix(button): ignore fill="none" paths on icon svgs * docs(Button): add temporary icon test Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../components/src/components/button/_button.scss | 2 +- .../components/src/components/button/_mixins.scss | 2 +- .../react/src/components/Button/Button-story.js | 14 +++++++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/components/src/components/button/_button.scss b/packages/components/src/components/button/_button.scss index fad2adaed0d4..a56edfcd9ba1 100644 --- a/packages/components/src/components/button/_button.scss +++ b/packages/components/src/components/button/_button.scss @@ -282,7 +282,7 @@ .#{$prefix}--btn--ghost.#{$prefix}--btn--icon-only .#{$prefix}--btn__icon - path:not([data-icon-path]), + path:not([data-icon-path]):not([fill='none']), .#{$prefix}--btn--ghost.#{$prefix}--btn--icon-only .#{$prefix}--btn__icon { fill: $icon-primary; } diff --git a/packages/components/src/components/button/_mixins.scss b/packages/components/src/components/button/_mixins.scss index 54f03eb375c0..1c5b5228c083 100644 --- a/packages/components/src/components/button/_mixins.scss +++ b/packages/components/src/components/button/_mixins.scss @@ -89,7 +89,7 @@ } .#{$prefix}--btn__icon, - .#{$prefix}--btn__icon path:not([data-icon-path]) { + .#{$prefix}--btn__icon path:not([data-icon-path]):not([fill='none']) { fill: $icon-color; } } diff --git a/packages/react/src/components/Button/Button-story.js b/packages/react/src/components/Button/Button-story.js index ab0c9dee6f1b..fd94e5e28dad 100644 --- a/packages/react/src/components/Button/Button-story.js +++ b/packages/react/src/components/Button/Button-story.js @@ -9,7 +9,13 @@ import React from 'react'; import { action } from '@storybook/addon-actions'; import { withKnobs, boolean, select, text } from '@storybook/addon-knobs'; import { iconAddSolid, iconSearch } from 'carbon-icons'; -import { Add16, AddFilled16, Search16 } from '@carbon/icons-react'; +import { + Add16, + AddFilled16, + Search16, + PlayOutlineFilled32, + PlayOutlineFilled16, +} from '@carbon/icons-react'; import Button from '../Button'; import ButtonSkeleton from '../Button/Button.Skeleton'; import ButtonSet from '../ButtonSet'; @@ -20,6 +26,10 @@ const icons = { 'Add (Add16 from `@carbon/icons-react`)': 'Add16', 'Add (Filled) (AddFilled16 from `@carbon/icons-react`)': 'AddFilled16', 'Search (Search16 from `@carbon/icons-react`)': 'Search16', + 'PlayOutlineFilled16 (PlayOutlineFilled16 from `@carbon/icons-react`)': + 'PlayOutlineFilled16', + 'PlayOutlineFilled32 (PlayOutlineFilled32 from `@carbon/icons-react`)': + 'PlayOutlineFilled32', }; const iconMap = { @@ -28,6 +38,8 @@ const iconMap = { Add16, AddFilled16, Search16, + PlayOutlineFilled16, + PlayOutlineFilled32, }; const kinds = { From 1f172df14d9c6d0e6ca05755e94aed28539e9881 Mon Sep 17 00:00:00 2001 From: Nicholas Fong Date: Mon, 10 May 2021 09:17:49 -0400 Subject: [PATCH 03/16] fix(BreadcrumbItem): do not overwrite child className (#8578) * fix(BreadcrumbItem): do not overwrite child className * refactor(BreadcrumbItem): reduce child classname to single line * refactor(BreadcrumbItem): remove unnecessary optional chaining for child classname check Co-authored-by: Josh Black --- packages/react/src/components/Breadcrumb/BreadcrumbItem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/Breadcrumb/BreadcrumbItem.js b/packages/react/src/components/Breadcrumb/BreadcrumbItem.js index 47ccb62763a9..4167fbb3aa5c 100644 --- a/packages/react/src/components/Breadcrumb/BreadcrumbItem.js +++ b/packages/react/src/components/Breadcrumb/BreadcrumbItem.js @@ -67,7 +67,7 @@ const BreadcrumbItem = React.forwardRef(function BreadcrumbItem(
  • {React.cloneElement(children, { 'aria-current': ariaCurrent, - className: `${prefix}--link`, + className: cx(`${prefix}--link`, children.props.className), })}
    • ); From 102f5169c4e4046ad4058ad86f2d6e077ca7b678 Mon Sep 17 00:00:00 2001 From: Scott Strubberg Date: Mon, 10 May 2021 11:11:57 -0500 Subject: [PATCH 04/16] docs: broken link to developer handbook (#8631) --- .github/CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 797da1c0fe2d..6e651d8bf593 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -270,8 +270,8 @@ GitHub. 3. **Development environment:** If you haven't already, fork and clone whichever repo you want to contribute to. Then, create a new branch and add your contribution in it. Checkout our - [Developer Handbook](../developer-handbook.md) to read up on our best coding - practices and proper commit messages. + [Developer Handbook](../docs/developer-handbook.md) to read up on our best + coding practices and proper commit messages. 4. **Pull request:** Submit a PR. Be sure to fill out the template properly. 5. **Approval:** Get PR approved by design and developers, or make any necessary changes for approval. This process may be quick or take a few iterations of From e4583e106781e5e394904d97a62c39881d23723b Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 10 May 2021 10:31:58 -0700 Subject: [PATCH 05/16] feat(TooltipIcon): support `renderIcon` (#8612) * feat(TooltipIcon): support `renderIcon` * docs(TooltipIcon): add `renderIcon` knob * chore: update snapshots Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../__snapshots__/PublicAPI-test.js.snap | 14 ++++++- .../TooltipIcon/TooltipIcon-story.js | 39 +++++++++++++------ .../src/components/TooltipIcon/TooltipIcon.js | 11 +++++- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 02c6146a3665..1b4effa1e48e 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -6471,7 +6471,6 @@ Map { "type": "oneOf", }, "children": Object { - "isRequired": true, "type": "node", }, "className": Object { @@ -6506,6 +6505,19 @@ Map { "onMouseLeave": Object { "type": "func", }, + "renderIcon": Object { + "args": Array [ + Array [ + Object { + "type": "func", + }, + Object { + "type": "object", + }, + ], + ], + "type": "oneOfType", + }, "tooltipText": Object { "isRequired": true, "type": "node", diff --git a/packages/react/src/components/TooltipIcon/TooltipIcon-story.js b/packages/react/src/components/TooltipIcon/TooltipIcon-story.js index 2283d8d7d5c6..c7430f250877 100644 --- a/packages/react/src/components/TooltipIcon/TooltipIcon-story.js +++ b/packages/react/src/components/TooltipIcon/TooltipIcon-story.js @@ -6,7 +6,7 @@ */ import React from 'react'; -import { Filter16 } from '@carbon/icons-react'; +import { Add16, AddFilled16, Filter16, Search16 } from '@carbon/icons-react'; import { action } from '@storybook/addon-actions'; import { withKnobs, select, text } from '@storybook/addon-knobs'; import TooltipIcon from '../TooltipIcon'; @@ -25,12 +25,31 @@ const alignments = { 'End (end)': 'end', }; -const props = () => ({ - direction: select('Tooltip direction (direction)', directions, 'bottom'), - align: select('Tooltip alignment (align)', alignments, 'center'), - tooltipText: text('Tooltip content (tooltipText)', 'Filter'), - onClick: action('onClick'), -}); +const icons = { + 'Add (Add16 from `@carbon/icons-react`)': 'Add16', + 'Add (Filled) (AddFilled16 from `@carbon/icons-react`)': 'AddFilled16', + 'Filter (Filter16 from `@carbon/icons-react`)': 'Filter16', + 'Search (Search16 from `@carbon/icons-react`)': 'Search16', +}; + +const iconMap = { + Add16, + AddFilled16, + Filter16, + Search16, +}; + +const props = () => { + const iconToUse = iconMap[select('Icon (icon)', icons, 'Filter16')]; + + return { + direction: select('Tooltip direction (direction)', directions, 'bottom'), + align: select('Tooltip alignment (align)', alignments, 'center'), + renderIcon: !iconToUse || iconToUse.svgData ? undefined : iconToUse, + tooltipText: text('Tooltip content (tooltipText)', 'Filter'), + onClick: action('onClick'), + }; +}; export default { title: 'Components/TooltipIcon', @@ -44,11 +63,7 @@ export default { }, }; -export const Default = () => ( - - - -); +export const Default = () => ; Default.storyName = 'default'; diff --git a/packages/react/src/components/TooltipIcon/TooltipIcon.js b/packages/react/src/components/TooltipIcon/TooltipIcon.js index acc4aabe18c6..6eb636a7089b 100644 --- a/packages/react/src/components/TooltipIcon/TooltipIcon.js +++ b/packages/react/src/components/TooltipIcon/TooltipIcon.js @@ -27,6 +27,7 @@ const TooltipIcon = ({ onFocus, onMouseEnter, onMouseLeave, + renderIcon: IconElement, tooltipText, ...rest }) => { @@ -132,7 +133,8 @@ const TooltipIcon = ({ id={tooltipId}> {tooltipText} - {children} + {IconElement && } + {!IconElement && children} ); }; @@ -148,7 +150,7 @@ TooltipIcon.propTypes = { * Specify an icon as children that will be used as the tooltip trigger. This * can be an icon from our Icon component, or a custom SVG element. */ - children: PropTypes.node.isRequired, + children: PropTypes.node, /** * Specify an optional className to be applied to the trigger node @@ -191,6 +193,11 @@ TooltipIcon.propTypes = { */ onMouseLeave: PropTypes.func, + /** + * Function called to override icon rendering. + */ + renderIcon: PropTypes.oneOfType([PropTypes.func, PropTypes.object]), + /** * Provide the ARIA label for the tooltip. * TODO: rename this prop (will be a breaking change) From 3a030892b60904ff215b5ccbd8bfcbea9d5058ca Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Mon, 10 May 2021 12:30:37 -0700 Subject: [PATCH 06/16] fix(Tooltip): remove pointer from TooltipDefinition, add disabled TooltipIcon (#8577) * fix(TooltipDefinition): remove pointer from tooltip * fix(TooltipDefinition): explicitly set cursor to default * fix(TooltipIcon): prevent pointer cursor on nointeractive variant * chore(tests): update snapshots * feat(TooltipIcon): add disabled support * chore(tests): update snapshots * docs(TooltipIcon): add playground story --- .../src/components/tooltip/_tooltip.scss | 9 ++-- .../components/src/globals/scss/_tooltip.scss | 6 ++- .../__snapshots__/PublicAPI-test.js.snap | 3 ++ .../TooltipIcon/TooltipIcon-story.js | 53 ++++++++++++++----- .../src/components/TooltipIcon/TooltipIcon.js | 35 ++++++++---- .../__snapshots__/TooltipIcon-test.js.snap | 10 ++++ 6 files changed, 88 insertions(+), 28 deletions(-) diff --git a/packages/components/src/components/tooltip/_tooltip.scss b/packages/components/src/components/tooltip/_tooltip.scss index 0f4541d726a4..f6d9a96932b6 100644 --- a/packages/components/src/components/tooltip/_tooltip.scss +++ b/packages/components/src/components/tooltip/_tooltip.scss @@ -151,8 +151,6 @@ color: $text-primary; &:hover { - cursor: pointer; - + .#{$prefix}--tooltip--definition__top, + .#{$prefix}--tooltip--definition__bottom { display: block; @@ -182,7 +180,6 @@ margin-top: $carbon--spacing-04; background: $background-inverse; border-radius: rem(2px); - cursor: pointer; pointer-events: none; p { @@ -343,7 +340,6 @@ display: inline-flex; align-items: center; - cursor: pointer; font-size: 1rem; &:focus { @@ -353,6 +349,11 @@ } } + // Disabled styles + .#{$prefix}--tooltip__trigger:not(.#{$prefix}--btn--icon-only)[disabled] svg { + fill: $icon-disabled; + } + .#{$prefix}--tooltip__label .#{$prefix}--tooltip__trigger { // Override `margin: 0` from button-reset mixin margin-left: $carbon--spacing-03; diff --git a/packages/components/src/globals/scss/_tooltip.scss b/packages/components/src/globals/scss/_tooltip.scss index 0ef3a447c7bd..45558fc1a312 100644 --- a/packages/components/src/globals/scss/_tooltip.scss +++ b/packages/components/src/globals/scss/_tooltip.scss @@ -78,13 +78,17 @@ display: inline-flex; overflow: visible; align-items: center; - cursor: pointer; &:focus { @include focus-outline('border'); } + @if $tooltip-type == 'definition' { + cursor: default; + } + @if $tooltip-type == 'icon' { + cursor: pointer; &:focus { outline: 1px solid transparent; diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 1b4effa1e48e..581065ede35e 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -6487,6 +6487,9 @@ Map { ], "type": "oneOf", }, + "disabled": Object { + "type": "bool", + }, "id": Object { "type": "string", }, diff --git a/packages/react/src/components/TooltipIcon/TooltipIcon-story.js b/packages/react/src/components/TooltipIcon/TooltipIcon-story.js index c7430f250877..2752946a3260 100644 --- a/packages/react/src/components/TooltipIcon/TooltipIcon-story.js +++ b/packages/react/src/components/TooltipIcon/TooltipIcon-story.js @@ -6,9 +6,15 @@ */ import React from 'react'; -import { Add16, AddFilled16, Filter16, Search16 } from '@carbon/icons-react'; +import { + Add16, + AddFilled16, + Filter16, + Search16, + Information16, +} from '@carbon/icons-react'; import { action } from '@storybook/addon-actions'; -import { withKnobs, select, text } from '@storybook/addon-knobs'; +import { withKnobs, select, boolean, text } from '@storybook/addon-knobs'; import TooltipIcon from '../TooltipIcon'; import mdx from './TooltipIcon.mdx'; @@ -43,6 +49,7 @@ const props = () => { const iconToUse = iconMap[select('Icon (icon)', icons, 'Filter16')]; return { + disabled: boolean('Disabled (disabled)', false), direction: select('Tooltip direction (direction)', directions, 'bottom'), align: select('Tooltip alignment (align)', alignments, 'center'), renderIcon: !iconToUse || iconToUse.svgData ? undefined : iconToUse, @@ -63,16 +70,34 @@ export default { }, }; -export const Default = () => ; +export const Default = () => ( +
    + + +
    +); -Default.storyName = 'default'; - -Default.parameters = { - info: { - text: ` - Icon tooltip is for short single line of text describing an icon. - Icon tooltip does not use any JavaScript. No label should be added to this variation. - If there are actions a user can take in the tooltip (e.g. a link or a button), use interactive tooltip. - `, - }, -}; +export const Playground = () => ( +
    + +
    +); diff --git a/packages/react/src/components/TooltipIcon/TooltipIcon.js b/packages/react/src/components/TooltipIcon/TooltipIcon.js index 6eb636a7089b..4645a5908f1c 100644 --- a/packages/react/src/components/TooltipIcon/TooltipIcon.js +++ b/packages/react/src/components/TooltipIcon/TooltipIcon.js @@ -21,6 +21,7 @@ const TooltipIcon = ({ className, children, direction, + disabled, align, onClick, onBlur, @@ -44,7 +45,7 @@ const TooltipIcon = ({ { [`${prefix}--tooltip--${direction}`]: direction, [`${prefix}--tooltip--align-${align}`]: align, - [`${prefix}--tooltip--hidden`]: !allowTooltipVisibility, + [`${prefix}--tooltip--hidden`]: !allowTooltipVisibility || disabled, [`${prefix}--tooltip--visible`]: isHovered, } ); @@ -74,17 +75,19 @@ const TooltipIcon = ({ }; const handleMouseEnter = (evt) => { - setIsHovered(true); - tooltipTimeout.current && clearTimeout(tooltipTimeout.current); + if (!disabled) { + setIsHovered(true); + tooltipTimeout.current && clearTimeout(tooltipTimeout.current); - if (evt.target === tooltipRef.current) { - setAllowTooltipVisibility(true); - return; - } + if (evt.target === tooltipRef.current) { + setAllowTooltipVisibility(true); + return; + } - closeTooltips(evt); + closeTooltips(evt); - setAllowTooltipVisibility(true); + setAllowTooltipVisibility(true); + } }; const handleMouseLeave = () => { @@ -115,8 +118,17 @@ const TooltipIcon = ({ return () => document.removeEventListener('keydown', handleEscKeyDown); }, []); + let cursorStyle; + if (disabled) { + cursorStyle = 'not-allowed'; + } else { + cursorStyle = onClick ? 'pointer' : 'default'; + } + return ( +   {!regularProps.kind.includes('danger') && ( - + <> + +   + + )}
    { ); }; +export const ExpressiveButtons = () => { + return ( + <> +
    + +
    +
    + +
    +
    + +
    +
    + +
    +
    +
    +
    + + + + +
    + + ); +}; + export const Skeleton = () => (
    diff --git a/packages/react/src/components/Button/Button.js b/packages/react/src/components/Button/Button.js index fe0ed52f354a..f45c2299c035 100644 --- a/packages/react/src/components/Button/Button.js +++ b/packages/react/src/components/Button/Button.js @@ -28,6 +28,7 @@ const Button = React.forwardRef(function Button( size, kind, href, + isExpressive, isSelected, tabIndex, type, @@ -122,14 +123,19 @@ const Button = React.forwardRef(function Button( const buttonClasses = classNames(className, { [`${prefix}--btn`]: true, - [`${prefix}--btn--sm`]: size === 'small' || size === 'sm' || small, - [`${prefix}--btn--md`]: size === 'field' || size === 'md', + [`${prefix}--btn--sm`]: + (size === 'small' && !isExpressive) || + (size === 'sm' && !isExpressive) || + (small && !isExpressive), + [`${prefix}--btn--md`]: + (size === 'field' && !isExpressive) || (size === 'md' && !isExpressive), // V11: change lg to xl [`${prefix}--btn--lg`]: enabled ? size === 'xl' : size === 'lg', // V11: change xl to 2xl [`${prefix}--btn--xl`]: enabled ? size === '2xl' : size === 'xl', [`${prefix}--btn--${kind}`]: kind, [`${prefix}--btn--disabled`]: disabled, + [`${prefix}--btn--expressive`]: isExpressive, [`${prefix}--tooltip--hidden`]: hasIconOnly && !allowTooltipVisibility, [`${prefix}--tooltip--visible`]: isHovered, [`${prefix}--btn--icon-only`]: hasIconOnly, @@ -275,6 +281,11 @@ Button.propTypes = { return undefined; }, + /** + * Specify whether the Button is expressive, or not + */ + isExpressive: PropTypes.bool, + /** * Specify whether the Button is currently selected */ @@ -383,6 +394,7 @@ Button.defaultProps = { dangerDescription: 'danger', tooltipAlignment: 'center', tooltipPosition: 'top', + isExpressive: false, }; export default Button; diff --git a/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap b/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap index dc5628a821a3..3b97bed5e5cc 100644 --- a/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap +++ b/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap @@ -1887,6 +1887,7 @@ exports[`DataTable should render 1`] = `