Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #149 - It is possible to select more than maxSelectedItems when #158

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions src/components/multi_select_state.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { PureComponent } from "react";
import {
getSelectedByAllItems,
getSelectedItemsOutsideInterval,
getAvailableIntervalForSelection,
filterUnselectedByIds,
findItem,
getMinMaxIndexes,
Expand Down Expand Up @@ -66,16 +68,32 @@ const withMultiSelectState = WrappedComponent =>
}

handleMultiSelection(index) {
const { items, isLocked } = this.props;
const { filteredItems, firstItemShiftSelected } = this.state;
const { items, isLocked, maxSelectedItems } = this.props;
const {
filteredItems,
firstItemShiftSelected,
selectedItems
} = this.state;

const initialInterval = getMinMaxIndexes(index, firstItemShiftSelected);
const outsideIntervalSelectedItems = getSelectedItemsOutsideInterval(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all this logic should happen just when the Shift pressed, right?
also, I prefer to extract all the logic to the utils.
the MinMax function will give the interval and will do all the necessary calculation in it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, I believe that it looks better, thanks for your feedback and please review again

items,
selectedItems,
initialInterval
);
const interval = getAvailableIntervalForSelection(
initialInterval,
outsideIntervalSelectedItems.length,
maxSelectedItems,
firstItemShiftSelected
);

const interval = getMinMaxIndexes(index, firstItemShiftSelected);
const newSelectedItems = items.filter(
(item, index) =>
(isWithin(index, interval) &&
!isLocked(item) &&
findItem(item, filteredItems)) ||
findItem(item, this.state.selectedItems)
findItem(item, outsideIntervalSelectedItems)
);
const newFilteredSelectedItems = this.getNewFilteredSelectedItems(
newSelectedItems
Expand Down
44 changes: 44 additions & 0 deletions src/components/multi_select_state_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,50 @@ export const getMinMaxIndexes = (currentIndex, firstItemShiftSelected) =>
? { minIndex: currentIndex, maxIndex: firstItemShiftSelected }
: { minIndex: firstItemShiftSelected, maxIndex: currentIndex };

export const getAvailableIntervalForSelection = (
interval,
outsideSelectedItems,
maxSelectedItems,
firstItemShiftSelected
) => {
const { minIndex, maxIndex } = interval;
const shouldBeSelect = outsideSelectedItems + (maxIndex - minIndex);
return maxSelectedItems > 0 && maxSelectedItems <= shouldBeSelect
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why you need to check > 0 ? is it not enough to do it like this:
maxSelectedItems && maxSelectedItems <= shouldBeSelect

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

? getFixedInterval(
interval,
outsideSelectedItems,
maxSelectedItems,
firstItemShiftSelected
)
: interval;
};

const getFixedInterval = (
{ minIndex, maxIndex },
outsideSelectedItems,
maxSelectedItems,
firstItemShiftSelected
) => ({
minIndex:
maxIndex === firstItemShiftSelected
? maxIndex - (maxSelectedItems - outsideSelectedItems - 1)
: minIndex,
maxIndex:
minIndex === firstItemShiftSelected
? minIndex + (maxSelectedItems - outsideSelectedItems - 1)
: maxIndex
});

export const getSelectedItemsOutsideInterval = (
sourceItems,
selectedItems,
interval
) =>
selectedItems.filter(selectedItem => {
const index = sourceItems.findIndex(item => item.id === selectedItem.id);
return !isWithin(index, interval) || selectedItem.disabled;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do u need the disable check? can u give me scenario?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot_5
Sure, please see attached screen

});

export const isWithin = (index, { minIndex, maxIndex }) =>
index >= minIndex && index <= maxIndex;

Expand Down
36 changes: 36 additions & 0 deletions tests/components/multi_select_state.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,4 +715,40 @@ describe("withMultiSelectState", () => {
items[4]
]);
});

test("case select with shift items where shouldBeSelect > maxSelectedItems", () => {
const ConditionalComponent = withMultiSelectState(CustomComponent);
const items = [ITEM_1, ITEM_2, ITEM_3, ITEM_4, ITEM_12];

const wrapper = shallow(
<ConditionalComponent
items={items}
selectedItems={[ITEM_1]}
maxSelectedItems={3}
/>
);
wrapper.props().selectItem(EVENT_WITH_SHIFT, ITEM_12.id);
wrapper.update();
wrapper.props().selectItem(EVENT_WITH_SHIFT, ITEM_3.id);
wrapper.update();
expect(wrapper.prop("selectedItems")).toEqual([ITEM_1, ITEM_4, ITEM_12]);
});

test("case select with shift items where shouldBeSelect < maxSelectedItems", () => {
const ConditionalComponent = withMultiSelectState(CustomComponent);
const items = [ITEM_1, ITEM_2, ITEM_3, ITEM_4];

const wrapper = shallow(
<ConditionalComponent
items={items}
selectedItems={[]}
maxSelectedItems={4}
/>
);
wrapper.props().selectItem(EVENT_WITH_SHIFT, ITEM_1.id);
wrapper.update();
wrapper.props().selectItem(EVENT_WITH_SHIFT, ITEM_3.id);
wrapper.update();
expect(wrapper.prop("selectedItems")).toEqual([ITEM_1, ITEM_2, ITEM_3]);
});
});
52 changes: 51 additions & 1 deletion tests/components/multi_select_state_utils.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {
filterUnselectedByIds,
getSelectedByAllItems
getSelectedByAllItems,
getSelectedItemsOutsideInterval,
getAvailableIntervalForSelection
} from "../../src/components/multi_select_state_utils";

const items = [
Expand Down Expand Up @@ -76,4 +78,52 @@ describe("testing utils for multi select state", () => {
);
expect(allSelectedItems).toEqual([]);
});

test("filter selectedItems that not in interval", () => {
const selectedItemsOutsideInterval = getSelectedItemsOutsideInterval(
items,
selectedItems,
{ minIndex: 1, maxIndex: 3 }
);
expect(selectedItemsOutsideInterval.length).toEqual(1);
expect(selectedItemsOutsideInterval[0]).toEqual(selectedItems[0]);
});

test("filter itemsToSelect that not in interval", () => {
const selectedItemsOutsideInterval = getSelectedItemsOutsideInterval(
items,
itemsToSelect,
{ minIndex: 0, maxIndex: 3 }
);
expect(selectedItemsOutsideInterval.length).toEqual(0);
});

test("available interval when minIndex == firstItemShiftSelected", () => {
const initialInterval = { minIndex: 0, maxIndex: 3 };
const maxSelectedItems = 3;
const selectedItemsOutsideInterval = 0;
const firstItemShiftSelected = 0;
const availableInterval = getAvailableIntervalForSelection(
initialInterval,
selectedItemsOutsideInterval,
maxSelectedItems,
firstItemShiftSelected
);
expect(availableInterval).toEqual({ minIndex: 0, maxIndex: 2 });
});

test("available interval when maxIndex == firstItemShiftSelected", () => {
const initialInterval = { minIndex: 1, maxIndex: 3 };
const maxSelectedItems = 3;
const selectedItemsOutsideInterval = 1;
const firstItemShiftSelected = 3;

const availableInterval = getAvailableIntervalForSelection(
initialInterval,
selectedItemsOutsideInterval,
maxSelectedItems,
firstItemShiftSelected
);
expect(availableInterval).toEqual({ minIndex: 2, maxIndex: 3 });
});
});