nandorojo / zeego

Menus for React (Native) done right.
https://zeego.dev
MIT License
1.42k stars 41 forks source link

Trigger adding extra `<View />` disrupts flex layout. #48

Closed Joehoel closed 10 months ago

Joehoel commented 1 year ago

Related to #41

zeego version: 1.5.2

Trying to create a grid layout where every item has a context menu with the following code:

<Animated.ScrollView
  contentContainerStyle={{
    gap: 8,
    flexWrap: "wrap",
    flexDirection: "row",
  }}
>
  {psalmen.map(psalm => (
    <ContextMenu.Root key={psalm.nummer}>
      <ContextMenu.Trigger>
        <TouchableOpacity
          className="aspect-square grow basis-16 items-center justify-center rounded bg-tertiary/20 p-4 dark:bg-tertiary/70"
          onPress={() => onPressPsalm(psalm)}
        >
          <Text className="font-bold text-dark-blue text-2xl dark:text-white">
            {psalm.nummer}
          </Text>
        </TouchableOpacity>
      </ContextMenu.Trigger>
      <ContextMenu.Content>
        {psalm.verzen.map((vers, idx) => (
          <ContextMenu.Item
            key={`vers-${vers.nummer}-${idx}`}
            onSelect={() => onVersPress(vers)}
          >
            <ContextMenu.ItemTitle>{getVersTitle(vers, false)}</ContextMenu.ItemTitle>
            <ContextMenu.ItemIcon
              ios={{
                hierarchicalColor: "red",
                name: isFavorite(vers) ? "heart.fill" : undefined,
              }}
            />
          </ContextMenu.Item>
        ))}
      </ContextMenu.Content>
    </ContextMenu.Root>
  ))}
</Animated.ScrollView>

The result of this code is this:

Screenshots

As seen in the screenshots the items in the grid are taking up the full with of the container which is (I think) a result of <ContextMenu.Trigger /> adding an extra <View /> to the tree.

Any suggestions on how to resolve this or maybe a different approach?

Reproduction: https://github.com/Joehoel/zeego-repro

nandorojo commented 1 year ago

Interesting, did you try adding style to the trigger? perhaps we could add asChild to it

Joehoel commented 1 year ago

Did you try adding style to the trigger?

Yeah I tried that but that makes the whole square dissapear.

Perhaps we could add asChild to it

That would be great!

nandorojo commented 1 year ago

could you try making a custom trigger using the create() function that just returns your touchable? this would mirror the asChild behavior

Joehoel commented 1 year ago

Still invisible

const ContextMenuTrigger = ContextMenu.create(TouchableOpacity, "Trigger");
<Animated.ScrollView
  contentContainerStyle={{
    paddingTop: 172 + insets.top,
    paddingBottom: 172 + insets.bottom,
    paddingHorizontal: 24,
    gap: 8,
    flexWrap: "wrap",
    flexDirection: "row",
  }}
>
  {psalmen.map(psalm => (
    <ContextMenu.Root key={psalm.nummer}>
      <ContextMenuTrigger className="aspect-square grow basis-16 items-center justify-center rounded bg-tertiary/20 p-4 dark:bg-tertiary/70">
        <Text className="font-bold text-dark-blue text-2xl dark:text-white">
          {psalm.nummer}
        </Text>
      </ContextMenuTrigger>
      <ContextMenu.Content>
        {psalm.verzen.map((vers, idx) => (
          <ContextMenu.Item
            key={`vers-${vers.nummer}-${idx}`}
            onSelect={() => onVersPress(vers, 1)}
          >
            <ContextMenu.ItemTitle>{getVersTitle(vers, false)}</ContextMenu.ItemTitle>
            <ContextMenu.ItemIcon
              ios={{
                hierarchicalColor: "red",
                name: isFavorite(vers) ? "heart.fill" : undefined,
              }}
            />
          </ContextMenu.Item>
        ))}
      </ContextMenu.Content>
    </ContextMenu.Root>
  ))}
</Animated.ScrollView>
Screenshot
nandorojo commented 1 year ago

that’s weird…and if you use pressable?

nandorojo commented 1 year ago

are you sure it can receive className here?

nandorojo commented 1 year ago

can you create a fully wrapped component that applies classname inside of it and just forwards down children inside of create?

Joehoel commented 1 year ago

Like this?

const ContextMenuTrigger = ContextMenu.create(
  ({ children }: MenuTriggerProps) => (
    <TouchableOpacity className="aspect-square grow basis-16 items-center justify-center rounded bg-tertiary/20 p-4 dark:bg-tertiary/70">
      {children}
    </TouchableOpacity>
  ),
  "Trigger"
);
Screenshot
Joehoel commented 1 year ago

that’s weird…and if you use pressable?

Same behaviour

Joehoel commented 1 year ago

@nandorojo Here's a repro: https://github.com/Joehoel/zeego-repro

nandorojo commented 10 months ago

You can now use asChild on the Trigger. Please let me know if this faces issues.