Skip to content

Commit

Permalink
Fix <Dropdown> multi-value bug (#374)
Browse files Browse the repository at this point in the history
* Fix multi-value bug

* Add tests

* Update dropdown.jest.js

Co-authored-by: Orr Gottlieb <60314759+orrgottlieb@users.noreply.github.com>
  • Loading branch information
sahariko and orrgottlieb authored Dec 5, 2021
1 parent 7f0b9bb commit 801a398
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 15 deletions.
5 changes: 4 additions & 1 deletion src/components/Dropdown/Dropdown.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ const Dropdown = ({
const isControlled = !!customValue;
const selectedOptions = customValue ?? selected;
const selectedOptionsMap = useMemo(
() => selectedOptions.reduce((acc, option) => ({ ...acc, [option.value]: option }), {}),
() =>
Array.isArray(selectedOptions)
? selectedOptions.reduce((acc, option) => ({ ...acc, [option.value]: option }), {})
: {},
[selectedOptions]
);
const value = multi ? selectedOptions : customValue;
Expand Down
12 changes: 8 additions & 4 deletions src/components/Dropdown/__tests__/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ export default class DropdownDriver {
this.renderResult = render(<Dropdown {...this.props} />);
}

rerender() {
this.renderResult = this.renderResult.rerender();
}

ensureRendered() {
if (!this.renderResult) {
this.render();
Expand Down Expand Up @@ -65,6 +61,10 @@ export default class DropdownDriver {
);
}

get singleValueText() {
return this.renderResult.container.querySelector("[class*='singleValue']").innerHTML;
}

focusInput() {
this.ensureRendered();

Expand Down Expand Up @@ -184,6 +184,10 @@ export default class DropdownDriver {
return this.setProp("value", value);
}

withOnChange(onChange) {
return this.setProp("onChange", onChange);
}

withOnOptionSelect(onOptionSelect) {
return this.setProp("onOptionSelect", onOptionSelect);
}
Expand Down
53 changes: 43 additions & 10 deletions src/components/Dropdown/__tests__/dropdown.jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,39 @@ describe("Dropdown", () => {
});
});

describe("Controlled", () => {
let component;

beforeEach(() => {
component = new DropdownDriver().withOptions().withValue(mockOptions[0]);
});

it("Should display the provided value", () => {
component.render();

expect(component.singleValueText).toBe("Ocean");
});

it("Should not change value internally", () => {
component.selectOption(2);
component.render();

expect(component.singleValueText).toBe("Ocean");
});

it("Should change displayed value when passed a new value", () => {
component.withOnChange(option => {
component.props.value = option;
});

component.render();
component.selectOption(2);
component.render();

expect(component.singleValueText).toBe("Purple");
});
});

describe("multi", () => {
let component;

Expand Down Expand Up @@ -125,44 +158,44 @@ describe("Dropdown", () => {
component.selectOption(0);

component.clearOptions();
component.rerender();
component.render();

expect(component.chips.values).toEqual([]);
});

describe("Controlled", () => {
let options;
let value;

beforeEach(() => {
options = [component.options[0], component.options[2]];
component.withValue(options);
value = [component.options[0], component.options[2]];
component.withValue(value);
});

it("Should support selecting multiple options", () => {
component.withOnOptionSelect(option => options.push(option));
component.withOnOptionSelect(option => value.push(option));

component.selectOption(3);
component.rerender();
component.render();

expect(component.chips.values).toEqual(["ocean", "purple", "red"]);
});

it("Should support removing options", () => {
component.withOnOptionRemove(() => options.pop());
component.withOnOptionRemove(() => value.pop());

component.removeOption(2);
component.rerender();
component.render();

expect(component.chips.values).toEqual(["ocean"]);
});

it("Should support clearing options", () => {
component.withOnClear(() => {
options.length = 0;
value.length = 0;
});

component.clearOptions();
component.rerender();
component.render();

expect(component.chips.values).toEqual([]);
});
Expand Down

0 comments on commit 801a398

Please sign in to comment.