Skip to content

chore: Add tag group component#29387

Merged
KelvinOm merged 10 commits intoreleasefrom
chore/group-tag
Dec 11, 2023
Merged

chore: Add tag group component#29387
KelvinOm merged 10 commits intoreleasefrom
chore/group-tag

Conversation

@jsartisan
Copy link
Contributor

@jsartisan jsartisan commented Dec 6, 2023

Fixes #29134

Summary by CodeRabbit

  • New Features

    • Introduced TagGroup and Tag components with features like label, description, error message, and tag removal options.
    • Added new stories for TagGroup to demonstrate various configurations and use cases.
  • Enhancements

    • Updated Switch component with improved validation logic.
    • Enhanced TextArea initialization to handle undefined default values more gracefully.
    • Added validationState property to HelpText and Field components to support form validation states.
  • Style Updates

    • Implemented new CSS styles for TagGroup and Tag components to improve UI consistency and interactivity.
  • Documentation

    • Expanded Storybook documentation with examples and usage guidelines for TagGroup and Tag components.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 6, 2023

Walkthrough

Based on the provided information, here's the updated content:

Walkthrough

The changes involve the creation of a new TagGroup component within the design system, which includes the Tag and TagGroup components, their styles, and storybook stories for documentation and examples. The TagGroup component is designed to handle and display a list of tags with additional features like labels, descriptions, and error messages. The Tag component allows for conditional rendering of a close button. These changes align with the creation of a component similar to the one described in the linked issue.

Changes

File Path Change Summary
.../TagGroup/index.ts Exported all entities from the "src" file.
.../TagGroup/src/Tag.tsx Added a new Tag React component.
.../TagGroup/src/TagGroup.tsx Introduced a new TagGroup component with additional features.
.../TagGroup/src/index.ts Exported TagGroup and Tag components.
.../TagGroup/src/styles.module.css Added CSS rules for styling tag groups and tags.
.../TagGroup/stories/TagGroup.stories.mdx Added storybook stories for the TagGroup component.
.../design-system/widgets/src/index.ts Exported the TagGroup component from the module.
.../headless/src/components/Field/HelpText.tsx Added validationState property to HelpTextProps.
.../headless/src/components/Field/src/Field.tsx Included a new validationState property in FieldProps.
.../headless/src/components/Field/src/HelpText.tsx Extended HelpTextProps to include validationState.
.../headless/src/components/Switch/Switch.tsx Modified SwitchProps to use Validation<boolean>.
.../headless/src/components/TextArea/src/TextArea.tsx Changed default value logic for inputValue in TextArea.

Assessment against linked issues

Objective Addressed Explanation
#29134: Create WDS tagGroup component The creation of TagGroup and Tag components, along with their styles and storybook stories, fulfills the objective of creating a TagGroup component similar to the one in the provided link.

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

@github-actions github-actions bot added WDS team Anvil Pod Issue related to Anvil project skip-changelog Adding this label to a PR prevents it from being listed in the changelog labels Dec 6, 2023

# Tag Group

Switch Group is a group of checkboxes that can be selected together.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The title "Switch Group" seems to be a copy-paste error and should be corrected to "Tag Group" to match the context of the component being documented.

- Switch Group is a group of checkboxes that can be selected together.
+ Tag Group is a group of tags that can be interacted with.

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
Switch Group is a group of checkboxes that can be selected together.
Tag Group is a group of tags that can be interacted with.


# Single Selection Mode

Switch Group can be configured to allow only one selection at a time with the `selectionMode` prop.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference to "Switch Group" should be updated to "Tag Group" to maintain consistency with the component being described.

- Switch Group can be configured to allow only one selection at a time with the `selectionMode` prop.
+ Tag Group can be configured to allow only one selection at a time with the `selectionMode` prop.

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
Switch Group can be configured to allow only one selection at a time with the `selectionMode` prop.
Tag Group can be configured to allow only one selection at a time with the `selectionMode` prop.


# Multiple Selection Mode

Switch Group can be configured to allow multiple selections at a time with the `selectionMode` prop with value `multiple`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference to "Switch Group" should be corrected to "Tag Group" to accurately describe the component's functionality.

- Switch Group can be configured to allow multiple selections at a time with the `selectionMode` prop with value `multiple`.
+ Tag Group can be configured to allow multiple selections at a time with the `selectionMode` prop with value `multiple`.

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
Switch Group can be configured to allow multiple selections at a time with the `selectionMode` prop with value `multiple`.
Tag Group can be configured to allow multiple selections at a time with the `selectionMode` prop with value `multiple`.

@jsartisan jsartisan requested a review from KelvinOm December 6, 2023 12:00
<HeadlessTag textValue={textValue} {...props} className={styles["tag"]}>
{({ allowsRemoving }) => (
<>
<span className={getTypographyClassName("footnote")}>{children}</span>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we pass className to the tag directly? Can we avoid adding span?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

</HeadlessText>
)}
{Boolean(errorMessage) && (
<HeadlessText elementType="div" slot="errorMessage">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each HeadlessText component will create an additional DOM node. Is it necessary? Can we use just WDS Text component in the same way? If not, then may be we can throw WDS typography classes into the spectrum component.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

{children}
</HeadlessTagList>
{Boolean(description) && (
<HeadlessText elementType="div" slot="description">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two components ( HeadlessText and Text ) are required because we need slot feature of HeadlessText and we need styles from our Text.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore. This is fixed.

</HeadlessText>
)}
{Boolean(errorMessage) && (
<HeadlessText elementType="div" slot="errorMessage">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<HeadlessTag textValue={textValue} {...props} className={styles["tag"]}>
{({ allowsRemoving }) => (
<>
<span className={getTypographyClassName("footnote")}>{children}</span>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

KelvinOm
KelvinOm previously approved these changes Dec 7, 2023
{...wrapperProps}
className={wrapperClassName}
data-disabled={isDisabled ? "" : undefined}
data-disabled={Boolean(isDisabled) ? "" : undefined}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data-disabled attribute should be a boolean value, but it's being set to an empty string or undefined. It should be set to true or false directly.

- data-disabled={Boolean(isDisabled) ? "" : undefined}
+ data-disabled={isDisabled}

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
data-disabled={Boolean(isDisabled) ? "" : undefined}
data-disabled={isDisabled}

@jsartisan jsartisan requested a review from KelvinOm December 8, 2023 11:14
"@blueprintjs/icons": "3.22.0",
"@types/react": "^17.0.2",
"postcss": "8.4.31",
"@react-types/shared": "3.19.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

data-testid="create-workspace-form__name"
name="name"
placeholder="Name"
// @ts-expect-error Type mismatch
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this relate to us?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Anvil Pod Issue related to Anvil project skip-changelog Adding this label to a PR prevents it from being listed in the changelog WDS team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create WDS tagGroup component

2 participants