grouped-checkboxes icon indicating copy to clipboard operation
grouped-checkboxes copied to clipboard

It'd be nice to be able to shift-click a selection of a large CheckboxGroup

Open BerendPronk opened this issue 6 years ago • 3 comments

Title is self explanatory

BerendPronk avatar Oct 07 '19 11:10 BerendPronk

Not a good description, but thanks for the suggestion.

I will be working on a shift-click feature, but want feedback on my solution. There are some issues that make this hard to develop, currently there is no order in how checkboxes are stored in the map.

POC branch: shift-click CodeSandbox example: https://codesandbox.io/s/tender-tharp-tnysj

As a proof of concept I created a function that loops through the children of the CheckboxGroup.

const getChildCheckboxes = (childNodes: ReactNode[]): string[] => {
  const checkboxList: string[] = [];

  if (!Array.isArray(childNodes)){
    childNodes = [childNodes];
  }

  childNodes.forEach((node => {
    if (objectIsCheckbox(node)) {
      checkboxList.push(node.props.id);
    } else if (objectHasChildren(node)) {
      checkboxList.push(
        ...getChildCheckboxes(node.props.children)
      );
    }
  }));

  return checkboxList;
};

getChildCheckboxes(children as ReactNode[] || [])

To (un)check the checkboxes within the range I created this function, where lastToggledId is the previously toggled checkbox.

const toggleShiftGroup = (key: string): void => {
    const endCheckbox = checkboxes.get(key);

    if (!endCheckbox || lastToggledId === undefined) {
      return;
    }

    const start = orderedIdList.indexOf(lastToggledId);
    const end = orderedIdList.indexOf(key);
    const toggleCheckboxes = orderedIdList.slice(start < end ? start : end, (end > start ? end : start) + 1);

    toggleCheckboxes.forEach((id) => {
      const toggleCheckbox = checkboxes.get(id);
      if (toggleCheckbox) {
        toggleCheckbox.setIsChecked(endCheckbox.isChecked || false);
      }
    });

    onCheckboxChange(key);
  };

It seems to be working but I'm not sure about performance and should be tested carefully.

AlexLisenkov avatar Dec 11 '19 10:12 AlexLisenkov

Well if Im only commenting on the readability of the logic, the getChildCheckboxes could be refactored to use flatMap

const getChildCheckboxes = (childNodes: ReactNode[]): string[] => {
  if (!Array.isArray(childNodes)){
    childNodes = [childNodes];
  }

  return childNodes.flatMap(node => {
    if (objectIsCheckbox(node)) {
      return [node.props.id];
    } if (objectHasChildren(node)) {
      return getChildCheckboxes(node.props.children);
    }
  });
};

And the orderedIdList.slice() could be refactored to use Math.min and Math.max statement

const toggleCheckboxes = orderedIdList.slice(Math.min(start, end), Math.max(end, start) + 1);

brokenhappy avatar Dec 12 '19 10:12 brokenhappy

Thanks for the suggestion. Unfortunately after some testing I found out that iterating though children prop does not work when a subcomponent renders a Checkbox that is not part of the children prop (should've guesses that).

I am thinking about another solution, maybe iterate though the dom tree. Could trigger the algo when the checkbox map changes (debounced), reindexing the ordered array. Not sure how to decide the implementation of objectIsCheckbox yet, could just check if the node is a checkbox containing an id.

AlexLisenkov avatar Dec 12 '19 11:12 AlexLisenkov