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(Tile): add slug prop to SelectableTile and ExpandableTile #15222

Merged
merged 13 commits into from
Nov 28, 2023

Conversation

tw15egan
Copy link
Collaborator

@tw15egan tw15egan commented Nov 16, 2023

Closes #15187

Adds in the slug prop for Tile, ClickableTile, SelectableTile, and ExpandableTile

Changelog

New

  • slug prop added to `Tile components listed above.
  • slug prop for ClickableTile is boolean only, as it will just render an SVG of the hollow slug since it cannot be interactive. @aagonzales I just pulled the SVG from the figma file and hardcoded it in. Do we have any plans to add this to the icon library, or is this fine for now?
  • New tokens to handle the transition between linear gradients when hovering or selecting a tile. Can probably be reduced when we revisit tokens / token naming.

Changed

  • Refactored slug.scss to use the new @mixin callout-gradient. This generates the background gradient for the Tile, as well as the hover and selected states. @include callout-gradient('hover');

Testing / Reviewing

Go to Tille --> Slug Test and review that the Slug renders in the correct place, the slug callout renders as expected, and that the hover / selected states of ClickableTile and SelectableTile are correct

Copy link

netlify bot commented Nov 16, 2023

Deploy Preview for v11-carbon-react ready!

Name Link
🔨 Latest commit f562ea4
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/656640e959443f00080f9646
😎 Deploy Preview https://deploy-preview-15222--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tw15egan
Copy link
Collaborator Author

tw15egan commented Nov 16, 2023

@aagonzales few observations:

  • I haven't done the static dot grid, but I can try and add what I have from the explorations if we want that.
  • ClickableTile currently does not support CTA inside as per guidance. Not sure if we can do this from an a11y perspective, but can take a deeper look
  • Should the basic Tile support the slug?
  • When the tile contains an interactive item, we take away the full tile hover in ExpandableTile. This will affect the hover styles in the spec.

Let me know how it looks and what else you notice.

@aagonzales
Copy link
Member

aagonzales commented Nov 16, 2023

@tw15egan

  • Don't worry about the dot grid with any of these components for the moment.
  • That's a good point about clickable tile. I might need to bring that up with Gower, thy and the AI folks
  • Yes, basic tile should support the slug (if possible)
  • I'll look into the expandable tile issue

@aagonzales
Copy link
Member

aagonzales commented Nov 17, 2023

@tw15egan For clickable tile let's do this.

  • Use the hollow slug and disable the tooltip, it shouldn't need explainability as clickable tile is normally just a link-out type of action instead of an option.
  • Added a spec for the hover values, this will need new tokens I think.
  • Light and dark themes specs here
image

@aagonzales
Copy link
Member

@tw15egan Selectable tile will need the same hover states and then it also has a selected state change as well.

image

@aagonzales
Copy link
Member

I'm also wondering if it would be easy to add an optional border radius prop for the callout as tile designs.

image

@tw15egan
Copy link
Collaborator Author

tw15egan commented Nov 20, 2023

@aagonzales just pushed a bunch of changes, including a bunch of new tokens to handle the gradient shift on hover / selected. WIth regards to the rounded edges, will this always be enabled if slug is present? Or can the slug be present and not have rounded edges? Can you have rounded edges and not have a slug? It's easy to do all of these things, but just want to make sure I guard it to match the intended use-cases

@tw15egan tw15egan requested a review from aagonzales November 20, 2023 20:30
@alisonjoseph
Copy link
Member

This is looking great @tw15egan! Do we need to adjust the padding on the right of the title to account for the slug?
Screenshot 2023-11-21 at 10 39 39 AM

@tw15egan
Copy link
Collaborator Author

@alisonjoseph Hmm the title in there is just for demo purposes... I can fix it here, but not too sure how we can go about systematically accounting for the slug there. This is where I wish you could do rounded corner padding in components! It would really help with this and other components like TextArea and CodeSnippet. Any ideas?

@alisonjoseph
Copy link
Member

@tw15egan oh yea, duh I knew that. That's tricky, we may just have to leave that up the to the teams building out the details. I don't know that we'd want to pad the entire tile over that much, especially the SelectableTile which already has padding to account for the checkbox.

Copy link
Contributor

@guidari guidari left a comment

Choose a reason for hiding this comment

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

LGTM! I didn't caught any issue. 🚀

@aagonzales
Copy link
Member

@tw15egan Ok this may change in the future but for now can we do this for the rounded corners:

  • Allowed: Slug with square corners
  • Allowed: Slug with rounded corners
  • Allowed: No slug with square corners
  • Not allowed: No slug with rounded corners
image

@tw15egan
Copy link
Collaborator Author

@aagonzales updated 👍🏻

Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

Looking good, just one thing. On hover with the rounded corner prop, the hover background still has squared corners. Also the same for the selected state
image

image

@tw15egan
Copy link
Collaborator Author

🤦🏻 whoops! Fixed it @aagonzales 👍🏻

Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

Ship it!

@tw15egan tw15egan added this pull request to the merge queue Nov 28, 2023
Merged via the queue into carbon-design-system:main with commit 3dd58b6 Nov 28, 2023
18 checks passed
@tw15egan tw15egan deleted the tile-slug-prop branch November 28, 2023 22:34
@carbon-automation
Copy link
Contributor

Hey there! v11.44.0 was just released that references this issue/PR.

danoro96 pushed a commit to danoro96/carbon that referenced this pull request Jan 18, 2024
…arbon-design-system#15222)

* docs(Tile): scaffold slug story, add demo styles

* feat(Tile): add slug to SelectableTile, ExpandableTile

* test(snapshot): update snapshots

* feat(Tile): add slug prop to Tile

* test(snapshot): update snapshots

* feat(Tile): add new tokens, add hover, selected styles

* test(snapshot): update snapshots

* feat(Tile): add new prop for rounded corners

* style(Tile): fix border-radius on pseudo elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add AI enhancements to React Tile
4 participants