ariakit icon indicating copy to clipboard operation
ariakit copied to clipboard

Unexpected behavior from `CheckboxCheck` with custom children when unchecked

Open bengry opened this issue 1 year ago • 5 comments

In a case where supplying a custom <CheckboxCheck> where you want to render something even when the item is unchecked (e.g. an empty checkbox), this won't work:

<Ariakit.CheckboxCheck
	checked={false} // realistically this value would come from a store of some kind
>
  <MyCheckbox />
</Ariakit.CheckboxCheck>

In this case the checkbox would only render when checked=true.

I tried to work around this using render:

<Ariakit.CheckboxCheck
	checked={false} // realistically this value would come from a store of some kind
	render={<MyCheckbox />}
/>

But then you get duplicated checkmarks when checked={true}.

So I tried this workaround:

<Ariakit.CheckboxCheck
	checked={false} // realistically this value would come from a store of some kind
	render={<MyCheckbox />}
>
{null}
</Ariakit.CheckboxCheck>

Expecting to override the default children and avoid the duplicated checkboxes, but that doesn't work because Ariakit checked if children is truthy, rather than just !== undefined.

This is of course still workaroundable by using a function for the render prop and omitting children, or passing in an empty Fragment as CheckboxCheck's children, but that seems like a hack more than anything.

I'd expect Ariakit to only supply the default SVG checkmark when children are not specified at all, not just when they're falsey.

bengry avatar May 22 '24 14:05 bengry

Hi @bengry! Could you please provide a StackBlitz or CodeSandbox with the code you're trying to implement and its current behavior? This will help determine if CheckboxCheck is indeed the right component for your needs and if this is a bug report or feature request.

diegohaz avatar May 22 '24 23:05 diegohaz

Hi @bengry! Could you please provide a StackBlitz or CodeSandbox with the code you're trying to implement and its current behavior? This will help determine if CheckboxCheck is indeed the right component for your needs and if this is a bug report or feature request.

Sure, you can see my use-case demoed here. This is a rough equivalent I think of my production app in this case, copied from the MenuItemCheckbox example in the docs.

What I'm trying to achieve is to render [ ] when the items are not checked, and [x] when they are. You can play around with the code a bit, but I could not get my wanted behavior to work.

bengry avatar May 24 '24 08:05 bengry

It seems that CheckboxCheck is not suitable for your use case. Simply get the checked state from the menu store and render your custom checkmark without CheckboxCheck:

export const MenuItemCheckbox = forwardRef<
  HTMLDivElement,
  MenuItemCheckboxProps
>(function MenuItemCheckbox(props, ref) {
  const menu = Ariakit.useMenuContext()!;

  const checked = menu.useState((state) => {
    // Ariakit should probably provide an isChecked/useChecked helper to simplify
    // this implementation
    const name = props.name;
    const value = props.value;
    const storeValue = state.values[name];
    if (value == null) return !!storeValue;
    if (!Array.isArray(storeValue)) return false;
    return storeValue.includes(`${value}`);
  })
  
  return (
    <Ariakit.MenuItemCheckbox
      ref={ref}
      {...props}
      className={clsx('menu-item', props.className)}
    >
      <VisualCheckbox checked={checked} />
      {props.children}
    </Ariakit.MenuItemCheckbox>
  );
});

diegohaz avatar May 24 '24 09:05 diegohaz

It seems that CheckboxCheck is not suitable for your use case. Simply get the checked state from the menu store and render your custom checkmark without CheckboxCheck:

Thanks, this makes sense. Isn't the checked prop that MenuItemCheckbox gets enough? Or is it not if we use the store to store the value? If that's the reason - the isChecked/useChecked helper you mentioned make sense and would be nice to have, but not crucial. Our current implementation up top of Ariakit relies on the consumer to pass a checked prop explicitly to this item.

bengry avatar May 25 '24 13:05 bengry

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 27 '25 02:04 stale[bot]