From e259aedf4ce8716a41c04c8ecfabfa23929f507c Mon Sep 17 00:00:00 2001 From: Dawid Date: Thu, 22 Apr 2021 10:59:37 +0200 Subject: [PATCH 1/2] fix(dialog): title wrapping --- src/components/dialog/dialog.style.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/dialog/dialog.style.js b/src/components/dialog/dialog.style.js index 9407026228..3892a0520e 100644 --- a/src/components/dialog/dialog.style.js +++ b/src/components/dialog/dialog.style.js @@ -91,7 +91,6 @@ const DialogTitleStyle = styled.div` color: ${({ theme }) => theme.text.color}; display: block; overflow: hidden; - white-space: nowrap; text-overflow: ellipsis; padding: ${({ hasSubtitle }) => !hasSubtitle && "4px 0px"}; } From 79b5d412ebf6d5bd59cb42a9a02c2367abd44aca Mon Sep 17 00:00:00 2001 From: Dawid Date: Wed, 28 Apr 2021 12:25:55 +0200 Subject: [PATCH 2/2] feat(dialog,dialog-full-screen): add help prop It allows to use `help` prop to add tooltlip message to our existing Heading in Dialog and Dialog Full Screen component Create ModalContext and save there `isAnimationComplete` state to use it inside of FocusTrap to help with setting focus to element after animation is done --- .../simpleColorPickerAction.feature | 12 ++--- .../test/advancedColorPickerNoIFrame.feature | 8 +-- .../simple-color-picker-steps.js | 11 ++++- .../focus-trap/focus-trap.component.js | 13 +++-- .../focus-trap/focus-trap.spec.js | 21 +++++--- .../advanced-color-picker.spec.js | 14 ++++-- .../dialog-full-screen.component.js | 4 ++ .../dialog-full-screen.spec.js | 23 +++++++++ .../dialog-full-screen.stories.mdx | 49 ++++++++++++++++++- .../dialog-full-screen.style.js | 10 +++- src/components/dialog/dialog.component.js | 4 ++ src/components/dialog/dialog.spec.js | 17 +++++-- src/components/dialog/dialog.stories.mdx | 46 ++++++++++++++++- src/components/dialog/dialog.style.js | 10 +++- .../modal/__snapshots__/modal.spec.js.snap | 2 + src/components/modal/modal.component.js | 11 ++++- 16 files changed, 219 insertions(+), 36 deletions(-) diff --git a/cypress/features/regression/experimental/simpleColorPickerAction.feature b/cypress/features/regression/experimental/simpleColorPickerAction.feature index e4f59e6f14..ae6aeb0183 100644 --- a/cypress/features/regression/experimental/simpleColorPickerAction.feature +++ b/cypress/features/regression/experimental/simpleColorPickerAction.feature @@ -18,33 +18,33 @@ Feature: Simple Color Picker component @positive Scenario: When on last color, going forward selects first color - When I press rightarrow on the 10 color + When I press rightarrow on the 10 color in IFrame Then Experimental Simple Color Picker 1 element was picked up @positive Scenario: When on first color, going backward selects last color - When I press leftarrow on the 1 color + When I press leftarrow on the 1 color in IFrame Then Experimental Simple Color Picker 10 element was picked up @positive Scenario: Left arrow moves selection left - When I press leftarrow on the 3 color + When I press leftarrow on the 3 color in IFrame Then Experimental Simple Color Picker 2 element was picked up @positive Scenario: Right arrow moves selection right - When I press rightarrow on the 3 color + When I press rightarrow on the 3 color in IFrame Then Experimental Simple Color Picker 4 element was picked up @positive Scenario: Up arrow moves selection up Given I select 6 color - When I press uparrow on the 6 color + When I press uparrow on the 6 color in IFrame Then Experimental Simple Color Picker 1 element was picked up @positive Scenario: Down arrow moves selection down - When I press downarrow on the 3 color + When I press downarrow on the 3 color in IFrame Then Experimental Simple Color Picker 8 element was picked up @positive diff --git a/cypress/features/regression/test/advancedColorPickerNoIFrame.feature b/cypress/features/regression/test/advancedColorPickerNoIFrame.feature index 68b1625a8f..cbe8f16f42 100644 --- a/cypress/features/regression/test/advancedColorPickerNoIFrame.feature +++ b/cypress/features/regression/test/advancedColorPickerNoIFrame.feature @@ -7,7 +7,7 @@ Feature: Advanced Color Picker component @positive Scenario Outline: moves selection - When I press "" onto focused element + When I press on the 7 color Then Simple Color element was picked up in noIframe Examples: | key | index | @@ -16,7 +16,7 @@ Feature: Advanced Color Picker component | uparrow | 2 | @positive - Scenario: Down arrow moves selection down - Given I press "uparrow" onto focused element - When I press "downarrow" onto focused element + Scenario: down arrow moves selection down + Given I press uparrow on the 7 color + When I press downarrow on the 2 color Then Simple Color 7 element was picked up in noIframe \ No newline at end of file diff --git a/cypress/support/step-definitions/simple-color-picker-steps.js b/cypress/support/step-definitions/simple-color-picker-steps.js index acf9f687be..953aae6dce 100644 --- a/cypress/support/step-definitions/simple-color-picker-steps.js +++ b/cypress/support/step-definitions/simple-color-picker-steps.js @@ -2,7 +2,10 @@ import { simpleColorPickerDiv, simpleColorPickerLegendNoIFrame, } from "../../locators/simple-color-picker"; -import { experimentalSimpleColorPickerInputInIframe } from "../../locators/advanced-color-picker/index"; +import { + experimentalSimpleColorPickerInputInIframe, + experimentalSimpleColorPickerInput, +} from "../../locators/advanced-color-picker/index"; import { getKnobsInput, commonDataElementInputPreviewNoIframe, @@ -39,13 +42,17 @@ When("I select {int} color", (index) => { experimentalSimpleColorPickerInputInIframe(index).click(); }); -When("I press {word} on the {int} color", (key, index) => { +When("I press {word} on the {int} color in IFrame", (key, index) => { experimentalSimpleColorPickerInputInIframe(index).trigger( "keydown", keyCode(key) ); }); +When("I press {word} on the {int} color", (key, index) => { + experimentalSimpleColorPickerInput(index).trigger("keydown", keyCode(key)); +}); + Then("It renders with all colors", () => { cy.fixture("simpleColorPicker.json").then(($json) => { for (let i = 0; i < $json.length; ++i) { diff --git a/src/__internal__/focus-trap/focus-trap.component.js b/src/__internal__/focus-trap/focus-trap.component.js index 5c726e6a29..984be1d00d 100644 --- a/src/__internal__/focus-trap/focus-trap.component.js +++ b/src/__internal__/focus-trap/focus-trap.component.js @@ -1,5 +1,6 @@ import { useCallback, + useContext, useEffect, useLayoutEffect, useRef, @@ -13,6 +14,7 @@ import { isRadio, setElementFocus, } from "./focus-trap-utils"; +import { ModalContext } from "../../components/modal/modal.component"; const FocusTrap = ({ children, @@ -25,7 +27,7 @@ const FocusTrap = ({ const [focusableElements, setFocusableElements] = useState(); const [firstElement, setFirstElement] = useState(); const [lastElement, setLastElement] = useState(); - + const { isAnimationComplete } = useContext(ModalContext); const hasNewInputs = useCallback( (candidate) => { if (!focusableElements || candidate.length !== focusableElements.length) { @@ -54,11 +56,16 @@ const FocusTrap = ({ }, [children, hasNewInputs, wrapperRef]); useEffect(() => { - if (autoFocus && firstOpen.current && (focusFirstElement || firstElement)) { + if ( + autoFocus && + firstOpen.current && + isAnimationComplete && + (focusFirstElement || firstElement) + ) { setElementFocus(focusFirstElement || firstElement); firstOpen.current = false; } - }, [autoFocus, firstElement, focusFirstElement]); + }, [autoFocus, firstElement, focusFirstElement, isAnimationComplete]); useEffect(() => { const trapFn = (ev) => { diff --git a/src/__internal__/focus-trap/focus-trap.spec.js b/src/__internal__/focus-trap/focus-trap.spec.js index 098ea4443e..3544e7f4ec 100644 --- a/src/__internal__/focus-trap/focus-trap.spec.js +++ b/src/__internal__/focus-trap/focus-trap.spec.js @@ -6,6 +6,7 @@ import { RadioButton, RadioButtonGroup, } from "../../__experimental__/components/radio-button"; +import { ModalContext } from "../../components/modal/modal.component"; jest.useFakeTimers(); @@ -14,11 +15,13 @@ const MockComponent = ({ children, ...rest }) => { const ref = useRef(); return ( - -
- {children} -
- + + +
+ {children} +
+ + ); }; @@ -419,9 +422,11 @@ describe("FocusTrap", () => { it("renders without wrapperRef provided", () => { expect(() => { mount( - -
Content
- + + +
Content
+ + ); }).not.toThrow(); }); diff --git a/src/components/advanced-color-picker/advanced-color-picker.spec.js b/src/components/advanced-color-picker/advanced-color-picker.spec.js index 9e236dbd5d..028ec41c5c 100644 --- a/src/components/advanced-color-picker/advanced-color-picker.spec.js +++ b/src/components/advanced-color-picker/advanced-color-picker.spec.js @@ -10,6 +10,7 @@ import { testStyledSystemMargin, } from "../../__spec_helper__/test-utils"; import { noThemeSnapshot } from "../../__spec_helper__/enzyme-snapshot-helper"; +import { ModalContext } from "../modal/modal.component"; jest.mock("../../utils/helpers/guid"); guid.mockImplementation(() => "guid-12345"); @@ -43,6 +44,8 @@ describe("AdvancedColorPicker", () => { function render(props = {}) { wrapper = mount(, { attachTo: htmlElement, + wrappingComponent: ModalContext.Provider, + wrappingComponentProps: { value: { isAnimationComplete: true } }, }); } @@ -105,10 +108,12 @@ describe("AdvancedColorPicker", () => { describe("when controlled", () => { describe("when dialog is open", () => { + jest.useFakeTimers(); describe("handleFocus focus trap callback", () => { describe("when key other than tab pressed", () => { it("should not change the focus", () => { render({ ...requiredProps, open: true }); + jest.runAllTimers(); const { defaultSimpleColor } = getElements(); expect(document.activeElement).toBe(defaultSimpleColor); @@ -132,6 +137,7 @@ describe("AdvancedColorPicker", () => { describe("when shift tab keys pressed on the selected color input", () => { it("should switch focus to the close button", () => { render({ ...requiredProps, open: true }); + jest.runAllTimers(); const { closeIcon, defaultSimpleColor } = getElements(); expect(document.activeElement).toBe(defaultSimpleColor); @@ -152,7 +158,7 @@ describe("AdvancedColorPicker", () => { }; render(extraProps); - + jest.runAllTimers(); const { simpleColors } = getElements(); expect(document.activeElement).toBe(simpleColors[0]); @@ -191,7 +197,7 @@ describe("AdvancedColorPicker", () => { }; render(extraProps); - + jest.runAllTimers(); const { simpleColors } = getElements(); expect(document.activeElement).toBe(simpleColors[7]); @@ -215,7 +221,7 @@ describe("AdvancedColorPicker", () => { }; render(extraProps); - + jest.runAllTimers(); const { simpleColors } = getElements(); expect(document.activeElement).toBe(simpleColors[7]); @@ -239,7 +245,7 @@ describe("AdvancedColorPicker", () => { }; render(extraProps); - + jest.runAllTimers(); const { simpleColors } = getElements(); expect(document.activeElement).toBe(simpleColors[7]); diff --git a/src/components/dialog-full-screen/dialog-full-screen.component.js b/src/components/dialog-full-screen/dialog-full-screen.component.js index 41196e9a32..53b3d4fe00 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.component.js +++ b/src/components/dialog-full-screen/dialog-full-screen.component.js @@ -24,6 +24,7 @@ const DialogFullScreen = ({ disableEscKey, onCancel, contentRef, + help, ...rest }) => { const dialogRef = useRef(); @@ -51,6 +52,7 @@ const DialogFullScreen = ({ subheader={subtitle} subtitleId="carbon-dialog-subtitle" divider={false} + help={help} /> ) : ( title @@ -114,6 +116,8 @@ DialogFullScreen.propTypes = { disableAutoFocus: PropTypes.bool, /** Determines if the Esc Key closes the Dialog */ disableEscKey: PropTypes.bool, + /** Adds Help tooltip to Header */ + help: PropTypes.string, /** remove padding from content */ disableContentPadding: PropTypes.bool, /** Child elements */ diff --git a/src/components/dialog-full-screen/dialog-full-screen.spec.js b/src/components/dialog-full-screen/dialog-full-screen.spec.js index f8e0e954a3..700a17f0a2 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.spec.js +++ b/src/components/dialog-full-screen/dialog-full-screen.spec.js @@ -13,6 +13,7 @@ import { assertStyleMatch } from "../../__spec_helper__/test-utils"; import IconButton from "../icon-button"; import StyledIconButton from "../icon-button/icon-button.style"; import { StyledHeader, StyledHeading } from "../heading/heading.style"; +import Help from "../help"; jest.mock("../../utils/helpers/guid"); @@ -67,6 +68,7 @@ describe("DialogFullScreen", () => { }); describe("autoFocus", () => { + jest.useFakeTimers(); it("should focus the first element by default", () => { mount( @@ -74,6 +76,8 @@ describe("DialogFullScreen", () => { ); + jest.runAllTimers(); + const firstFocusableElement = document.querySelector("input"); expect(document.activeElement).toBe(firstFocusableElement); }); @@ -85,6 +89,8 @@ describe("DialogFullScreen", () => { ); + jest.runAllTimers(); + const firstFocusableElement = document.querySelector("input"); expect(document.activeElement).not.toBe(firstFocusableElement); }); @@ -92,6 +98,7 @@ describe("DialogFullScreen", () => { describe("focusFirstElement", () => { it("should focus on the element passes as focusFirstElement prop", () => { + jest.useFakeTimers(); const Component = () => { const secondInputRef = useRef(null); return ( @@ -103,6 +110,8 @@ describe("DialogFullScreen", () => { }; mount(); + jest.runAllTimers(); + const secondFocusableElement = document.querySelectorAll("input")[1]; expect(document.activeElement).toEqual(secondFocusableElement); }); @@ -232,6 +241,20 @@ describe("DialogFullScreen", () => { expect(heading.props().title).toEqual("my custom heading"); }); }); + + describe("when prop help is passed", () => { + it("should render Help component", () => { + wrapper = mount( + + ); + + expect(wrapper.find(Help).exists()).toBe(true); + }); + }); }); describe("tags", () => { diff --git a/src/components/dialog-full-screen/dialog-full-screen.stories.mdx b/src/components/dialog-full-screen/dialog-full-screen.stories.mdx index 63a69e6a2b..ec730622e0 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.stories.mdx +++ b/src/components/dialog-full-screen/dialog-full-screen.stories.mdx @@ -1,4 +1,4 @@ -import { useState } from "react"; +import { useState, useRef } from "react"; import { Meta, Story, Preview, Props } from "@storybook/addon-docs/blocks"; import DialogFullScreen from "."; @@ -660,6 +660,53 @@ to have a possibility to manage active `Tabs` group +### With help + + + + {() => { + const [isOpen, setIsOpen] = useState(true); + return ( + <> + + setIsOpen(false)} + title="An example of a long header" + subtitle="Subtitle" + help="Some help text" + > +
setIsOpen(false)}>Cancel + } + saveButton={ + + } + > +
+ This is an example of a full screen Dialog with a Form as + content +
+ + + + + + + +
+ + ); + }} + + + ### With hidable header children diff --git a/src/components/dialog-full-screen/dialog-full-screen.style.js b/src/components/dialog-full-screen/dialog-full-screen.style.js index e2b62168b0..cd3d532b18 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.style.js +++ b/src/components/dialog-full-screen/dialog-full-screen.style.js @@ -3,7 +3,11 @@ import baseTheme from "../../style/themes/base"; import StyledContent from "./content.style"; import StyledIconButton from "../icon-button/icon-button.style"; import StyledFullScreenHeading from "../../__internal__/full-screen-heading/full-screen-heading.style"; -import { StyledHeader, StyledHeading } from "../heading/heading.style"; +import { + StyledHeader, + StyledHeaderContent, + StyledHeading, +} from "../heading/heading.style"; const StyledDialogFullScreen = styled.div` background-color: ${({ theme }) => theme.disabled.input}; @@ -16,6 +20,10 @@ const StyledDialogFullScreen = styled.div` display: flex; flex-direction: column; + ${StyledHeaderContent} { + align-items: baseline; + } + > ${StyledIconButton} { margin: 0; position: absolute; diff --git a/src/components/dialog/dialog.component.js b/src/components/dialog/dialog.component.js index bd2f85c7d5..aef9ba9b13 100644 --- a/src/components/dialog/dialog.component.js +++ b/src/components/dialog/dialog.component.js @@ -33,6 +33,7 @@ const Dialog = ({ showCloseIcon, bespokeFocusTrap, disableClose, + help, ...rest }) => { const dialogRef = useRef(); @@ -132,6 +133,7 @@ const Dialog = ({ subheader={subtitle} subtitleId="carbon-dialog-subtitle" divider={false} + help={help} /> ) : ( title @@ -227,6 +229,8 @@ Dialog.propTypes = { disableClose: PropTypes.bool, /** Allows developers to specify a specific height for the dialog. */ height: PropTypes.string, + /** Adds Help tooltip to Header */ + help: PropTypes.string, /** Title displayed at top of dialog */ title: PropTypes.oneOfType([PropTypes.string, PropTypes.object]), /** Subtitle displayed at top of dialog */ diff --git a/src/components/dialog/dialog.spec.js b/src/components/dialog/dialog.spec.js index 2df9a87a28..9237659a74 100644 --- a/src/components/dialog/dialog.spec.js +++ b/src/components/dialog/dialog.spec.js @@ -11,6 +11,7 @@ import { assertStyleMatch } from "../../__spec_helper__/test-utils"; import Form from "../form"; import { StyledFormFooter } from "../form/form.style"; import IconButton from "../icon-button"; +import Help from "../help"; describe("Dialog", () => { let onCancel; @@ -53,7 +54,7 @@ describe("Dialog", () => { it("does not bind if component is not open on mount", () => { wrapper = mount( - +
); @@ -74,7 +75,7 @@ describe("Dialog", () => { it("does not remove the event listener if it was not in use on unmount", () => { wrapper = mount( - +
); @@ -85,7 +86,7 @@ describe("Dialog", () => { it("adds event listeners on modal open", () => { wrapper = mount( - +
); @@ -273,6 +274,16 @@ describe("Dialog", () => { expect(wrapper.find(Heading).exists()).toBe(false); }); }); + + describe("when prop help is passed", () => { + it("should render Help component", () => { + wrapper = mount( + + ); + + expect(wrapper.find(Help).exists()).toBe(true); + }); + }); }); describe("render", () => { diff --git a/src/components/dialog/dialog.stories.mdx b/src/components/dialog/dialog.stories.mdx index 693c389d66..7ffeb91de2 100644 --- a/src/components/dialog/dialog.stories.mdx +++ b/src/components/dialog/dialog.stories.mdx @@ -1,4 +1,4 @@ -import { useState } from "react"; +import { useState, useRef } from "react"; import { Meta, Story, Preview, Props } from "@storybook/addon-docs/blocks"; import LinkTo from "@storybook/addon-links/react"; import Box from "../box"; @@ -168,6 +168,50 @@ When mixing editable and non-editable content, you can use the