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

[feat] <rh-chip> element #1456

Open
2 tasks
dcaryll opened this issue Jan 22, 2024 · 25 comments · May be fixed by #2198
Open
2 tasks

[feat] <rh-chip> element #1456

dcaryll opened this issue Jan 22, 2024 · 25 comments · May be fixed by #2198
Assignees
Labels
for dev Ready for development new component New component to be created

Comments

@dcaryll
Copy link
Collaborator

dcaryll commented Jan 22, 2024

Description

RHDS needs the have the ability to filter pieces of content using "chips". This is typically a single taxonomy way of filtering versus the multiple ways of filtering that a taxonomy box offers.

https://www.patternfly.org/components/chip/

Example in production on cloud.redhat.com/learn
Screenshot 2024-01-22 at 1 34 27 PM

Acceptance Criteria

  • Design done
  • Development done

Image

No response

Link to design doc

No response

Other resources

No response

@coreyvickery
Copy link
Collaborator

@dcaryll Just so you're aware, here are some concepts that are being explored and reviewed.

https://www.figma.com/proto/JbdmuSkfh0CuzqUZixb1UY?node-id=1-5&locale=en

@dcaryll
Copy link
Collaborator Author

dcaryll commented Feb 20, 2024 via email

@coreyvickery
Copy link
Collaborator

@markcaron @hellogreg I would like to get this done in Charmander.

@markcaron markcaron changed the title [feat] Chip(s) [feat] <rh-chip> element May 1, 2024
@bennypowers
Copy link
Member

pf-chip composes into pf-text-input for things like search. will rh-chip do the same? would that change it's semantics or usability? cc @adamjohnson

@adamjohnson
Copy link
Collaborator

pf-chip composes into pf-text-input for things like search. will rh-chip do the same?

Was the <pf-text-input> + <pf-chip> composition use case discussed elsewhere with regards to accessibility?

This is a common pattern seen on the web. Unfortunately, as far as I can tell, the APG Patterns don't have guidance for an input containing multiple selected items (correct me if I am wrong). Select2 is a classic library that does this but has had accessibility issues over the years.

IMO, we should steer users away from this UI pattern until the accessibility story becomes more solidified.

@coreyvickery
Copy link
Collaborator

coreyvickery commented May 3, 2024

@adamjohnson I think pf-chip is being deprecated in PatternFly 6. I am checking with the team about this.

[DEPRECATED FILE] is a comprehensive file of what I have been working on for rh-chip. I just need a few folks from other design teams to approve first before you get started.

Edit: This no longer applies, see comment below.

There are some explorations of smaller chips for inputs on the last page.

@coreyvickery
Copy link
Collaborator

@adamjohnson According to the PatternFly team, pf-chip is being replaced by pf-label.

If rh-chip needs to be used in inputs like search, maybe we can use rh-tag instead or add to rh-tag if needed, like a more compact version.

@zeroedin
Copy link
Collaborator

zeroedin commented May 6, 2024

@coreyvickery Just FYI had a short conversation with @eyevana about compact on rh-tag today, maybe get her use case for that as well?

@hellogreg hellogreg moved this from Backlog to In Progress 🟢 in Red Hat Design System May 16, 2024
@coreyvickery
Copy link
Collaborator

Update 5/22: Design leads will give feedback on this component on Tuesday, May 28. Marionne will present as Corey will be out.

@coreyvickery
Copy link
Collaborator

@markcaron Do we need anyone else to approve of the current design or can we proceed with development?

@markcaron markcaron added the for design Design work is requested label Oct 31, 2024
@markcaron
Copy link
Collaborator

@coreyvickery if design is good with it, then I'm good with it.

@coreyvickery coreyvickery removed their assignment Jan 13, 2025
@coreyvickery coreyvickery added for dev Ready for development and removed for design Design work is requested labels Jan 21, 2025
@markcaron markcaron assigned adamjohnson and unassigned coreyvickery Feb 7, 2025
@adamjohnson
Copy link
Collaborator

@coreyvickery:

Thanks for putting this design together. It's very helpful! I have a few questions for you.

I put together a little scratchpad demo to help clarify some things with Chip. There are obv lots of issues with spacing and it lacks variants and whatnot, but reference it when looking over my questions below:

  • Is Chip a radio or checkbox?
    • AKA: can you only select one chip at a time or multiple?
  • Does the "X" button deselect the chip?
  • Does clicking (not on the X) deselect the chip?
  • Does clicking the "X" remove the chip from the page?
  • What is "Overflow"?
    • I'm sorry, having trouble clarifying what the intent of that is from the design
  • Right now, there's no "View All" / uncheck all button/radio/checkbox. Is this intentional?
  • Whether or not these function like a radio or checkbox, they will still need to be paired with a <fieldset> + <legend> so that they have an accessible name. Can you add a <legend> to the Figma?

@coreyvickery
Copy link
Collaborator

@adamjohnson Thanks for the demo and writing these questions.

I'm going to check with Krystal one last time to make sure she has what she needs from a design standpoint before we continue with development. I should have an answer in a day or two.

@coreyvickery
Copy link
Collaborator

@adamjohnson Let's meet about the following after I talk with Krystal.

Whether or not these function like a radio or checkbox, they will still need to be paired with a

+ so that they have an accessible name. Can you add a to the Figma?

@bennypowers
Copy link
Member

bennypowers commented Feb 25, 2025

@adamjohnson @coreyvickery if we also create an <rh-chip-group> element, with a label slot/attr pair (see screenshot in OP i.e. the label is "Filter By:"), then we can handle the semantics internally

@marionnegp
Copy link
Collaborator

marionnegp commented Feb 26, 2025

@coreyvickery, I chatted with Krystal, Dustin, and Grady about chip and got some feedback about the visual design:

  • Close button looks kind of like it's part of the text because of the icon weight and proximity. What about using close-circle icon for clarity? (PF uses the close-circle-fill icon for their clickable labels, even though it deviates from both of our typical close buttons.)
  • I think the close button should be removed when it's not yet selected.
  • Do we need a disabled state? (Maybe this would be a feature we add later because I don't think we have a current use case for it.)

Re: question about it functioning like a radio button or a checkbox: It sounds like search would need a chip group that functions like a radio button. Search would also need the ability to remove individual chips from a group. Could we have a chip variant for each function? Unsure what we would name the variants. Having different variants might also help answer @adamjohnson's questions about what the close button does.

@marionnegp
Copy link
Collaborator

@adamjohnson @coreyvickery if we also create an element, with a label slot/attr pair (see screenshot in OP i.e. the label is "Filter By:"), then we can handle the semantics internally

@adamjohnson, would the "Filter by:" count as a <legend>?

@adamjohnson
Copy link
Collaborator

@coreyvickery: Thanks for this document! It provides some much-needed clarifications.

RE: Truncated Chip Text

Since the text is in the light DOM and can be styled by end users, we can make this a pattern. We can include documentation regarding text truncation while recommending a maximum character count and provide clear Do/Don’t examples. Generally, truncating text via CSS hides content, which can negatively impact users. IIRC, we had a similar discussion with rh-breadcrumbs.

Updates & Implementations

  • I’ll adjust the "Filter by:" <legend> to appear on the left side of the chips.
  • I’ll implement logic to show/hide the close icon based on whether a chip is selected.
  • I’ll add and style a disabled chip state.
  • I’ll ensure all states and colors align with the given specs (pending Marionne’s feedback).

Concern: Overflow Chip

One concern for this release is implementing the "Overflow" chip (e.g., "View all," "5 more," or "Show less"). While this pattern is straightforward in traditional implementations, working within the constraints of web components adds complexity. Given the short time frame before release—and considering I’ll be on PTO from March 6-11—delaying this feature to the next release may help us ship the component faster while maintaining quality. With Cubone scheduled for release on April 1st, ensuring enough time for development and review is critical.

Concern: radio Chip & Accessibility

I’ve been experimenting with making a chip a radio input, and I’m seeing similar accessibility issues that Arathy faced with pf-radio. Since each <rh-chip> is an individual radio input inside an encapsulated shadow DOM, the browser doesn’t recognize its sibling radio inputs.

When testing <rh-chip radio> with VO + Safari, Safari incorrectly announces "Radio, 1 of 1," and tabbing results in an improper focus order—failing WCAG SC 2.4.3, Focus Order (Level A). Given these challenges, I recommend postponing the radio functionality for the next release to ensure we get the accessibility implementation right and avoid creating barriers for users.

@coreyvickery
Copy link
Collaborator

@adamjohnson

Truncation

I'm fine with not truncating text.

Overflow

I'm fine with delaying this.

radio

Are you proposing no radio functionality, but checkbox instead? Or what do we do here?

Let me know if you need exact styling specs.

@marionnegp What's your pick of styling out of the four options? I personally like v1b.

@marionnegp
Copy link
Collaborator

@coreyvickery, yeah, I think v1b shows the clearest relationship between the unselected and selected chips.

@adamjohnson, I think I mistranscribed some notes when I said that search would need it to function like a radio button because search should allow for multiple chips to be selected/added to a group. Sorry about that!

@adamjohnson
Copy link
Collaborator

Are you proposing no radio functionality, but checkbox instead? Or what do we do here?

Yes, for now rh-chip will only have checkbox functionality due to the concerns outlined above.

I'll work on getting the colors synced from v1b and pushed up to Github. Thanks ya'll!

@adamjohnson adamjohnson linked a pull request Mar 5, 2025 that will close this issue
3 tasks
@adamjohnson adamjohnson linked a pull request Mar 5, 2025 that will close this issue
3 tasks
@coreyvickery
Copy link
Collaborator

Update 3/13: Docs are in progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for dev Ready for development new component New component to be created
Projects
Status: In Progress 🟢
Status: In Progress 🟢
Development

Successfully merging a pull request may close this issue.

9 participants