diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 380b2a7250..8a36238542 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -8,7 +8,7 @@ body: attributes: value: | ### ⚠️ This bug report is public - Please create a reproducible example using this [CodeSandbox template](https://codesandbox.io/s/carbon-quickstart-j5pb2). + Please create a reproducible example using one of our Codesandbox templates, using [Typescript](https://codesandbox.io/p/sandbox/carbon-quickstart-typescript-6sjfuz) or [Javascript](https://codesandbox.io/p/sandbox/carbon-quickstart-j5pb2) Please do not include any confidential or commercially sensitive information. --- @@ -34,7 +34,7 @@ body: attributes: label: CodeSandbox or Storybook URL description: >- - Please fork [this template](https://codesandbox.io/s/carbon-quickstart-j5pb2), and ensure you save your changes. + Please fork one of our templates ([Typescript](https://codesandbox.io/p/sandbox/carbon-quickstart-typescript-6sjfuz) or [Javascript](https://codesandbox.io/s/carbon-quickstart-j5pb2)), and ensure you save your changes. Alternatively, if your issue relates to [Storybook](https://carbon.sage.com) please link to the story. placeholder: https://codesandbox.io/s/carbon-quickstart-j5pb2 validations: diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml index 666efb9058..ad0c7fd8a5 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.yml +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -17,7 +17,8 @@ body: label: Desired behaviour description: | Please give a clear and concise description of the desired behaviour. - If applicable, add screenshots or screen recording of a [CodeSandbox](https://codesandbox.io/s/carbon-quickstart-j5pb2) to help explain the request. You can paste these directly into GitHub. + If applicable, add screenshots or screen recording of a CodeSandbox to help explain the request. You can paste these directly into GitHub. + We provide Codesandbox templates with Carbon and dependencies pre-loaded, using either [Typescript](https://codesandbox.io/p/sandbox/carbon-quickstart-typescript-6sjfuz) or [Javascript](https://codesandbox.io/p/sandbox/carbon-quickstart-j5pb2). validations: required: true - type: textarea @@ -39,7 +40,7 @@ body: attributes: label: CodeSandbox or Storybook URL description: >- - Please fork [this template](https://codesandbox.io/s/carbon-quickstart-j5pb2), and ensure you save your changes. + Please fork one of our templates ([Typescript](https://codesandbox.io/p/sandbox/carbon-quickstart-typescript-6sjfuz) or [Javascript](https://codesandbox.io/s/carbon-quickstart-j5pb2)), and ensure you save your changes. Alternatively, if your issue relates to [Storybook](https://carbon.sage.com) please link to the story. placeholder: https://codesandbox.io/s/carbon-quickstart-j5pb2 validations: diff --git a/CHANGELOG.md b/CHANGELOG.md index b9c3f63e7d..37715147d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +### [124.2.2](https://github.com/Sage/carbon/compare/v124.2.1...v124.2.2) (2023-12-11) + + +### Bug Fixes + +* **filterable-select:** ensure list does not reopen when user clicks option and openOnFocus prop set ([e143aaa](https://github.com/Sage/carbon/commit/e143aaad13c821a9ee6bfb9ce1a64494c81ce23b)), closes [#6462](https://github.com/Sage/carbon/issues/6462) + +### [124.2.1](https://github.com/Sage/carbon/compare/v124.2.0...v124.2.1) (2023-12-08) + + +### Bug Fixes + +* **textbox:** ensure no action is triggered when disabled or readOnly input or input icon is clicked ([0f028f9](https://github.com/Sage/carbon/commit/0f028f9a9e3e8268e978c9ccb1b94e7db84bcc9c)), closes [#6471](https://github.com/Sage/carbon/issues/6471) + ## [124.2.0](https://github.com/Sage/carbon/compare/v124.1.0...v124.2.0) (2023-12-06) diff --git a/cypress/components/select/filterable-select/filterable-select.cy.tsx b/cypress/components/select/filterable-select/filterable-select.cy.tsx index 4b8e8ebd81..d9698decdc 100644 --- a/cypress/components/select/filterable-select/filterable-select.cy.tsx +++ b/cypress/components/select/filterable-select/filterable-select.cy.tsx @@ -576,6 +576,34 @@ context("Tests for FilterableSelect component", () => { selectListWrapper().should("be.visible"); }); + it("should not reopen list when openOnFocus set and user selects an option via click", () => { + CypressMountWithProviders( + + ); + + commonDataElementInputPreview().focus(); + selectInput().should("have.attr", "aria-expanded", "true"); + selectListWrapper().should("be.visible"); + selectOption(positionOfElement("first")).click(); + selectListWrapper().should("not.be.visible"); + }); + + it("should open list when openOnFocus set, user selects an option via enter key and then input is blurred then focussed again", () => { + CypressMountWithProviders( + + ); + + commonDataElementInputPreview().focus(); + selectInput().should("have.attr", "aria-expanded", "true"); + selectListWrapper().should("be.visible"); + selectInput().realPress("ArrowDown"); + selectInput().realPress("Enter"); + selectListWrapper().should("not.be.visible"); + commonDataElementInputPreview().blur(); + commonDataElementInputPreview().focus(); + selectListWrapper().should("be.visible"); + }); + it("should check list is open when input is clicked and openOnFocus is set", () => { CypressMountWithProviders( diff --git a/package-lock.json b/package-lock.json index d9e30628c6..0ac557c556 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "carbon-react", - "version": "124.2.0", + "version": "124.2.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "carbon-react", - "version": "124.2.0", + "version": "124.2.2", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { diff --git a/package.json b/package.json index 56ba70f82e..e890120b43 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "carbon-react", - "version": "124.2.0", + "version": "124.2.2", "description": "A library of reusable React components for easily building user interfaces.", "files": [ "lib", diff --git a/src/components/date/date.component.tsx b/src/components/date/date.component.tsx index 206303ec99..2af91f4120 100644 --- a/src/components/date/date.component.tsx +++ b/src/components/date/date.component.tsx @@ -328,10 +328,6 @@ export const DateInput = React.forwardRef( const handleClick = ( ev: React.MouseEvent | React.KeyboardEvent ) => { - if (disabled || readOnly) { - return; - } - if (onClick) { onClick(ev); } @@ -340,10 +336,6 @@ export const DateInput = React.forwardRef( const handleMouseDown = (ev: React.MouseEvent) => { handleClickInside(); - if (disabled || readOnly) { - return; - } - if (setInputRefMap) { isBlurBlocked.current = true; } diff --git a/src/components/select/filterable-select/filterable-select.component.tsx b/src/components/select/filterable-select/filterable-select.component.tsx index d3ffb9dbd2..64bdb73242 100644 --- a/src/components/select/filterable-select/filterable-select.component.tsx +++ b/src/components/select/filterable-select/filterable-select.component.tsx @@ -153,6 +153,7 @@ export const FilterableSelect = React.forwardRef( label, }); const focusTimer = useRef>(null); + const openOnFocusFlagBlock = useRef(false); if (!deprecateInputRefWarnTriggered && inputRef) { deprecateInputRefWarnTriggered = true; @@ -478,12 +479,14 @@ export const FilterableSelect = React.forwardRef( setActiveDescendantId(selectedOptionId); if (selectionType !== "navigationKey") { + openOnFocusFlagBlock.current = !!openOnFocus; setOpen(false); textboxRef?.focus(); textboxRef?.select(); + openOnFocusFlagBlock.current = false; } }, - [textboxRef, triggerChange] + [textboxRef, triggerChange, openOnFocus] ); const onSelectListClose = useCallback(() => { @@ -521,6 +524,10 @@ export const FilterableSelect = React.forwardRef( clearTimeout(focusTimer.current); } + if (openOnFocusFlagBlock.current) { + return; + } + // we need to use a timeout here as there is a race condition when rendered in a modal // whereby the select list isn't visible when the select is auto focused straight away focusTimer.current = setTimeout(() => { diff --git a/src/components/select/filterable-select/filterable-select.spec.tsx b/src/components/select/filterable-select/filterable-select.spec.tsx index 49e71fa5d1..c9c26c3b1d 100644 --- a/src/components/select/filterable-select/filterable-select.spec.tsx +++ b/src/components/select/filterable-select/filterable-select.spec.tsx @@ -36,8 +36,8 @@ function getSelect(props: Partial = {}) { ); } -function renderSelect(props = {}, renderer = mount) { - return renderer(getSelect(props)); +function renderSelect(props = {}, renderer = mount, opts = {}) { + return renderer(getSelect(props), opts); } describe("FilterableSelect", () => { @@ -1097,7 +1097,7 @@ describe("FilterableSelect", () => { describe('when the "openOnFocus" prop is set', () => { describe("and the Textbox Input is focused", () => { - it("the SelectList should be rendered", () => { + it("should render the SelectList", () => { const wrapper = renderSelect({ openOnFocus: true }); act(() => { @@ -1109,6 +1109,38 @@ describe("FilterableSelect", () => { .forEach((option) => expect(option.getDOMNode()).toBeVisible()); }); + it("should not reopen the SelectList when a user selects and Option by clicking", () => { + const container = document.createElement("div"); + container.id = "enzymeContainer"; + document.body.appendChild(container); + + const wrapper = renderSelect({ openOnFocus: true }, mount, { + attachTo: document.getElementById("enzymeContainer"), + }); + + act(() => { + wrapper.find("input").simulate("focus"); + jest.runOnlyPendingTimers(); + }); + wrapper + .find(Option) + .forEach((option) => expect(option.getDOMNode()).toBeVisible()); + act(() => { + wrapper.find(SelectList).prop("onSelect")({ + value: "opt1", + text: "red", + selectionType: "click", + selectionConfirmed: true, + }); + }); + wrapper + .update() + .find(Option) + .forEach((option) => expect(option.getDOMNode()).not.toBeVisible()); + + container?.parentNode?.removeChild(container); + }); + describe.each(["readOnly", "disabled"])( 'with the "%s" prop passed', (prop) => { diff --git a/src/components/select/select-textbox/select-textbox.component.tsx b/src/components/select/select-textbox/select-textbox.component.tsx index dd6cc5e2c9..b246c5d26a 100644 --- a/src/components/select/select-textbox/select-textbox.component.tsx +++ b/src/components/select/select-textbox/select-textbox.component.tsx @@ -163,10 +163,6 @@ const SelectTextbox = React.forwardRef( function handleTextboxClick( event: React.MouseEvent | React.KeyboardEvent ) { - if (disabled || readOnly) { - return; - } - onClick?.(event as React.MouseEvent); } @@ -238,7 +234,7 @@ const SelectTextbox = React.forwardRef( { expect(callbackCount).toBe(1); }); + ["disabled", "readOnly"].forEach((propName) => { + test(`should not call onClick callback when a click event is triggered and input is ${propName}`, async ({ + mount, + page, + }) => { + let callbackCount = 0; + await mount( + { + callbackCount += 1; + }} + disabled={propName === "disabled"} + readOnly={propName === "readOnly"} + /> + ); + + await textboxInput(page).dispatchEvent("click"); + + expect(callbackCount).toBe(0); + }); + + test(`should not call onMouseDown callback when a click event is triggered and input is ${propName}`, async ({ + mount, + page, + }) => { + let callbackCount = 0; + await mount( + { + callbackCount += 1; + }} + disabled={propName === "disabled"} + readOnly={propName === "readOnly"} + /> + ); + + await textboxInput(page).dispatchEvent("mousedown"); + + expect(callbackCount).toBe(0); + }); + + test(`should not call iconOnMouseDown callback when a click event is triggered and input is ${propName}`, async ({ + mount, + page, + }) => { + let callbackCount = 0; + await mount( + { + callbackCount += 1; + }} + inputIcon="add" + disabled={propName === "disabled"} + readOnly={propName === "readOnly"} + /> + ); + + await getDataComponentByValue(page, "icon").click({ button: "left" }); + + expect(callbackCount).toBe(0); + }); + + test(`should not call iconOnClick callback when a click event is triggered and input is ${propName}`, async ({ + mount, + page, + }) => { + let callbackCount = 0; + await mount( + { + callbackCount += 1; + }} + inputIcon="add" + disabled={propName === "disabled"} + readOnly={propName === "readOnly"} + /> + ); + + await getDataComponentByValue(page, "icon").click(); + + expect(callbackCount).toBe(0); + }); + }); + test("should call onClick callback when a click event is triggered", async ({ mount, page, diff --git a/src/components/textbox/textbox.spec.tsx b/src/components/textbox/textbox.spec.tsx index 6f91557ee4..6b490f22ab 100644 --- a/src/components/textbox/textbox.spec.tsx +++ b/src/components/textbox/textbox.spec.tsx @@ -177,6 +177,7 @@ describe("Textbox", () => { expect(wrapper.find(InputPresentation).props().hasIcon).toBe(true); } ); + it("supports a separate onClick handler passing for the icon", () => { const onClick = jest.fn(); const iconOnClick = jest.fn(); @@ -197,6 +198,100 @@ describe("Textbox", () => { expect(onClick).not.toHaveBeenCalled(); }); + it.each(["disabled", "readOnly"])( + "does not call iconOnClick handler for the icon when input is %s", + (propName) => { + const onClick = jest.fn(); + const iconOnClick = jest.fn(); + + const wrapper = mount( + + normal children + + ); + const icon = wrapper.find(InputIconToggle); + icon.simulate("click"); + expect(iconOnClick).not.toHaveBeenCalled(); + expect(onClick).not.toHaveBeenCalled(); + } + ); + + it.each(["disabled", "readOnly"])( + "does not call iconOnMouseDown handler for the icon when input is %s", + (propName) => { + const onMouseDown = jest.fn(); + const iconOnMouseDown = jest.fn(); + + const wrapper = mount( + + normal children + + ); + const icon = wrapper.find(InputIconToggle); + icon.simulate("click"); + expect(iconOnMouseDown).not.toHaveBeenCalled(); + expect(onMouseDown).not.toHaveBeenCalled(); + } + ); + + it.each(["disabled", "readOnly"])( + "does not call onClick handler when input is %s and icon is clicked", + (propName) => { + const onClick = jest.fn(); + + const wrapper = mount( + + normal children + + ); + const icon = wrapper.find(InputIconToggle); + icon.simulate("click"); + expect(onClick).not.toHaveBeenCalled(); + } + ); + + it.each(["disabled", "readOnly"])( + "does not call onMouseDown handler when input is %s and icon is clicked", + (propName) => { + const onMouseDown = jest.fn(); + + const wrapper = mount( + + normal children + + ); + const icon = wrapper.find(InputIconToggle); + icon.simulate("click"); + expect(onMouseDown).not.toHaveBeenCalled(); + } + ); + describe("validation icon", () => { const validationTypes = ["error", "warning", "info"]; it.each(validationTypes)(