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

Add Chip and ChipGroup #327

Merged
merged 16 commits into from
Oct 10, 2024
Merged

Add Chip and ChipGroup #327

merged 16 commits into from
Oct 10, 2024

Conversation

AnnMarieW
Copy link
Collaborator

@AnnMarieW AnnMarieW commented Oct 5, 2024

Added 3 new components

  • dmc.ChipSingle Use for a single stand-alone chip

  • dmc.ChipGroup Use for multiple chips in a group. Can allow the user to set either one or multiple chips in the group using the multiple prop.

  • dmc.Chip Use in the children of the dmc.ChipGroup component. Do not use dmc.ChipSingle with dmc.ChipGroup

Note to reviewers:

There are three sample apps to demonstrate the features of these new components.

I created both the dmc.ChipSingle and dmc.Chip because when the checked prop is updated in the Chip then dmc.ChipGroup didn't work correctly. The dmc.ChipGroup uses the value prop to manage the state.

Is there a better way to do this rather than having both dmc.Chip and dmc.ChipSingle components?

Update --> Yes there was a better way -- see below. Solved by @BSd3v 🏆

Update --> Docs are ready to go 🎉 They will stay as a Draft PR until 0.14.6 is released snehilvj/dmc-docs#88

@AnnMarieW
Copy link
Collaborator Author

AnnMarieW commented Oct 7, 2024

Just trying to make the issue a little more clear:

From the Mantine docs, here is an example of a single controlled Chip:

import { useState } from 'react';
import { Chip } from '@mantine/core';

function Demo() {
  const [checked, setChecked] = useState(false);

  return (
    <Chip checked={checked} onChange={() => setChecked((v) => !v)}>
      My chip
    </Chip>
  );
}

In this example, the onChange function is applied directly to the Chip component.

However, in the Controlled Chip Group, the onChange function is set on the Chip.Group component. If the checked prop is set in the onChange prop in the individual Chip components, then the group does not work correctly:

function Multiple() {
  // Array of strings representing selected values when multiple is true
  const [value, setValue] = useState(['react']);

  return (
    <Chip.Group multiple value={value} onChange={setValue}>
      <Chip value="react">React</Chip>
      <Chip value="ng">Angular</Chip>
      <Chip value="svelte">Svelte</Chip>
      <Chip value="vue">Vue</Chip>
    </Chip.Group>
  );
}

I'm uncertain how to handle this in Dash without creating two separate Chip components: one that includes the onChange function for standalone use and another without it for use within the Chip.Group.

What I have so far works - I just think there must be a better way....

@AnnMarieW
Copy link
Collaborator Author

AnnMarieW commented Oct 8, 2024

Yes, there is a better way... Thanks for this suggestion @BSd3v

There is now just the Chip and ChipGroup components. When using the Chip as a single stand-alone component, set controlled=True This aligns nicely with the upstream Mantine docs: https://mantine.dev/core/chip/#controlled

const Chip = (props: Props) => {
    const {
        children,
        setProps,
        persistence,
        persisted_props,
        persistence_type,
        controlled,
        checked,
        ...others
    } = props;

    const onChange = (checked: boolean) => {
        setProps({ checked });
    };
    if (controlled) {
    return (
        <MantineChip checked={checked} onChange={onChange} {...others}>
            {children}
        </MantineChip>
    );
    } else {
     return (
        <MantineChip {...others}>
            {children}
        </MantineChip>
    );
    }
};

@AnnMarieW AnnMarieW changed the base branch from master to dev October 8, 2024 18:38
@AnnMarieW AnnMarieW changed the base branch from dev to master October 8, 2024 18:38
@AnnMarieW AnnMarieW marked this pull request as ready for review October 8, 2024 18:48
@AnnMarieW AnnMarieW requested a review from alexcjohnson October 8, 2024 18:49
Copy link

Generated link: snehilvj/dash-mantine-components-327

Copy link

Generated link: snehilvj/dash-mantine-components-327

Copy link

Generated link: snehilvj/dash-mantine-components-327

Copy link

Generated link: snehilvj/dash-mantine-components-327

Copy link

Generated link: snehilvj/dash-mantine-components-327

@AnnMarieW AnnMarieW merged commit 116f047 into snehilvj:master Oct 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant