Fix: missing checkmark icon on Android's CheckboxItem

[MarceloPrado]

Closes #45

This PR ensures the CheckboxItem component renders a checkmark on Android when the <ItemIndicator /> component is available. The implementation follows the composability patterns of Zeego:

1. Allow users to override the icon by passing a <ItemIcon /> component.
2. Allow users to hide the checkmark icon by not passing the <ItemIndicator /> component. See Notes, I believe this should also be the iOS behavior given what's written in the docs, but this might be a breaking change.
3. Renders the native checkmark icon when the <ItemIcon /> is available.

Code example

See the Screenshots section for the before/after of running this code:

<DropdownMenu.Root>
  <DropdownMenu.Trigger>
    <Button onPress={noop} variant="secondary">
      Default
    </Button>
  </DropdownMenu.Trigger>
  <DropdownMenu.Content>
    <DropdownMenu.Item key="1">
      <DropdownMenu.ItemTitle>Item + ItemIcon</DropdownMenu.ItemTitle>
      <DropdownMenu.ItemIcon androidIconName="btn_star" />
    </DropdownMenu.Item>

    {/* Expected: no checkmark icon */}
    <DropdownMenu.CheckboxItem key="2" value="off">
      <DropdownMenu.ItemTitle>CheckboxItem, off</DropdownMenu.ItemTitle>
    </DropdownMenu.CheckboxItem>

    {/* Expected: no checkmark icon */}
    <DropdownMenu.CheckboxItem key="3" value="on">
      <DropdownMenu.ItemTitle>CheckboxItem, on</DropdownMenu.ItemTitle>
    </DropdownMenu.CheckboxItem>

    {/* Expected: checkmark off icon */}
    <DropdownMenu.CheckboxItem key="4" value="off">
      <DropdownMenu.ItemTitle>
        CheckboxItem + ItemIndicator, off
      </DropdownMenu.ItemTitle>
      <DropdownMenu.ItemIndicator />
    </DropdownMenu.CheckboxItem>

    {/* Expected: checkmark on icon */}
    <DropdownMenu.CheckboxItem key="5" value="on">
      <DropdownMenu.ItemTitle>
        CheckboxItem + ItemIndicator, on
      </DropdownMenu.ItemTitle>
      <DropdownMenu.ItemIndicator />
    </DropdownMenu.CheckboxItem>

    {/* Expected: custom icon */}
    <DropdownMenu.CheckboxItem key="5" value="on">
      <DropdownMenu.ItemTitle>
        CheckboxItem + ItemIcon
      </DropdownMenu.ItemTitle>
      <DropdownMenu.ItemIcon androidIconName="btn_star" />
      <DropdownMenu.ItemIndicator />
    </DropdownMenu.CheckboxItem>
  </DropdownMenu.Content>
</DropdownMenu.Root>

Screenshots

	Before (Bug)	After (Fixed)
Android

Notes

My understanding from the ItemIndicator docs is that Android and iOS Menus should only show the native checkmarks if the <ItemIndicator /> component is rendered. However, this isn't the runtime behavior yet:

• iOS shows the native checkmark, irrespectively of whether ItemIndicator is rendered or not.
• Android (before my PR) didn't show any icons, with or without ItemIndicator

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
zeego-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 17, 2024 10:25pm

[nandorojo]

Nice! Can you ensure support for a Boolean type for value as well since this is also supported?

[nandorojo]

Judging by the Android docs, it seems like there's built-in checkbox support: https://developer.android.com/reference/android/view/MenuItem

I wonder if react-native-menu supports this, instead of using an icon to show the checked state.

[MarceloPrado]

Nice! Can you ensure support for a Boolean type for value as well since this is also supported?

I'm using the "normalized" version which should already cover the Boolean case. Does it make sense?

[nandorojo]

https://github.com/react-native-menu/menu

It looks like they have a MenuState at the bottom to leverage the native checkbox capabilities. This should be pretty close to the iOS approach I think...

[nandorojo]

Ah, got it, I just went off your comment showing the string props. Checking the code, looks like the boolean is indeed supported.

[MarceloPrado]

@nandorojo I think this is the closest we get - notice it's setting isVisible from MenuItem - we'd need to add an additional checked to MenuAttributes - it's a shame they don't forward the rest of attributes 😔