Skip to content

Commit

Permalink
Merge branch 'master' into pw_refactor_multiactionbutton
Browse files Browse the repository at this point in the history
  • Loading branch information
stephenogorman authored Dec 12, 2023
2 parents 6341968 + 1e01ae0 commit 04f637a
Show file tree
Hide file tree
Showing 13 changed files with 279 additions and 28 deletions.
4 changes: 2 additions & 2 deletions .github/ISSUE_TEMPLATE/bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
---
Expand All @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions .github/ISSUE_TEMPLATE/feature_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<stories.FilterableSelectComponent openOnFocus />
);

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(
<stories.FilterableSelectComponent openOnFocus />
);

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(
<stories.FilterableSelectComponent openOnFocus />
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
8 changes: 0 additions & 8 deletions src/components/date/date.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,6 @@ export const DateInput = React.forwardRef<HTMLInputElement, DateInputProps>(
const handleClick = (
ev: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>
) => {
if (disabled || readOnly) {
return;
}

if (onClick) {
onClick(ev);
}
Expand All @@ -340,10 +336,6 @@ export const DateInput = React.forwardRef<HTMLInputElement, DateInputProps>(
const handleMouseDown = (ev: React.MouseEvent<HTMLElement>) => {
handleClickInside();

if (disabled || readOnly) {
return;
}

if (setInputRefMap) {
isBlurBlocked.current = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export const FilterableSelect = React.forwardRef(
label,
});
const focusTimer = useRef<null | ReturnType<typeof setTimeout>>(null);
const openOnFocusFlagBlock = useRef(false);

if (!deprecateInputRefWarnTriggered && inputRef) {
deprecateInputRefWarnTriggered = true;
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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(() => {
Expand Down
38 changes: 35 additions & 3 deletions src/components/select/filterable-select/filterable-select.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ function getSelect(props: Partial<FilterableSelectProps> = {}) {
);
}

function renderSelect(props = {}, renderer = mount) {
return renderer(getSelect(props));
function renderSelect(props = {}, renderer = mount, opts = {}) {
return renderer(getSelect(props), opts);
}

describe("FilterableSelect", () => {
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,6 @@ const SelectTextbox = React.forwardRef(
function handleTextboxClick(
event: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>
) {
if (disabled || readOnly) {
return;
}

onClick?.(event as React.MouseEvent<HTMLInputElement>);
}

Expand Down Expand Up @@ -238,7 +234,7 @@ const SelectTextbox = React.forwardRef(
<SelectText
transparent={transparent}
placeholder={placeholder || l.select.placeholder()}
onClick={handleTextboxClick}
onClick={disabled || readOnly ? undefined : handleTextboxClick}
disabled={disabled}
readOnly={readOnly}
size={size}
Expand Down
10 changes: 6 additions & 4 deletions src/components/textbox/textbox.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,9 @@ export const Textbox = React.forwardRef(
onBlur={onBlur}
onChange={onChange}
onChangeDeferred={onChangeDeferred}
onClick={onClick}
onClick={disabled || readOnly ? undefined : onClick}
onFocus={onFocus}
onMouseDown={onMouseDown}
onMouseDown={disabled || readOnly ? undefined : onMouseDown}
placeholder={disabled || readOnly ? "" : placeholder}
readOnly={readOnly}
value={typeof formattedValue === "string" ? formattedValue : value}
Expand All @@ -299,8 +299,10 @@ export const Textbox = React.forwardRef(
iconTabIndex={iconTabIndex}
info={info}
inputIcon={inputIcon}
onClick={iconOnClick || onClick}
onMouseDown={iconOnMouseDown || onMouseDown}
onClick={disabled || readOnly ? undefined : iconOnClick || onClick}
onMouseDown={
disabled || readOnly ? undefined : iconOnMouseDown || onMouseDown
}
readOnly={readOnly}
size={size}
useValidationIcon={!(validationRedesignOptIn || validationOnLabel)}
Expand Down
84 changes: 84 additions & 0 deletions src/components/textbox/textbox.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,90 @@ test.describe("Event checks", () => {
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(
<TextboxComponent
onClick={() => {
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(
<TextboxComponent
onMouseDown={() => {
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(
<TextboxComponent
iconOnMouseDown={() => {
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(
<TextboxComponent
iconOnClick={() => {
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,
Expand Down
Loading

0 comments on commit 04f637a

Please sign in to comment.