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.

Sign up for GitHub

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

Jump to bottom

Dropdown::ListItem::Interactive - Add support for trailing icon #2042

Merged
merged 9 commits into from
Apr 11, 2024
Merged

Dropdown::ListItem::Interactive - Add support for trailing icon #2042

merged 9 commits into from
Apr 11, 2024

Conversation

didoo
Copy link
Contributor

@didoo didoo commented Apr 8, 2024 •
edited
Loading

📌 Summary

This PR introduces the support for an additional trailing icon to the Dropdown::ListItem::Interactive subcomponent

🛠️ Detailed description

In this PR I have:

  • added support for @trailingIcon in the Dropdown::ListItem::Interactive sub-component
  • added demo of combinations of @icon/@trailingIcon in showcase
  • updated integration tests for Dropdown::ListItem::Interactive
  • updated documentation

🔗 External links

Jira ticket: https://hashicorp.atlassian.net/browse/HDS-3205

👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

@didoo didoo requested review from majedelass and a team April 8, 2024 10:27
@vercel Vercel
Copy link

vercel bot commented Apr 8, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview Apr 11, 2024 8:46am
hds-website ✅ Ready (Inspect) Visit Preview Apr 11, 2024 8:46am

alex-ju
alex-ju approved these changes Apr 8, 2024
View reviewed changes
Copy link
Member

@alex-ju alex-ju 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! Will there be a follow-up PR to document @trailingIcon in website?

@didoo
Copy link
Contributor Author

didoo commented Apr 8, 2024

Looking good! Will there be a follow-up PR to document @trailingIcon in website?

Yes, I have to sync with @majedelass to understand if he has already started a branch, and if he prefers to use a distinct PR/branch or we can use directly this one.

majedelass
majedelass approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@majedelass majedelass left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good to me 👍 also figma matches this :)

(no PR created for the docs yet)

@majedelass
Updated 'Icon usage' section with a new bullet point mentioning that …
9b6a9b3 
…if youre using a list item interactive that a trailing icon can be used to denote that the link is external. Also added an example to this in the example too.
@hashibot-hds hashibot-hds added the docs-website Content updates to the documentation website label Apr 9, 2024
@didoo
Copy link
Contributor Author

didoo commented Apr 9, 2024

Looking good! Will there be a follow-up PR to document @trailingIcon in website?

@alex-ju we've added the documentation updated directly in this PR.

alex-ju
alex-ju approved these changes Apr 9, 2024
View reviewed changes
Copy link
Member

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Left a few thoughts, but good to be merged

website/docs/components/dropdown/partials/code/how-to-use.md Outdated Show resolved Hide resolved
website/docs/components/dropdown/partials/code/component-api.md Outdated Show resolved Hide resolved
jorytindall
jorytindall approved these changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@jorytindall jorytindall left a comment

Choose a reason for hiding this comment

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

One small non-blocking comment, otherwise 👍

website/docs/components/dropdown/partials/guidelines/guidelines.md Outdated Show resolved Hide resolved
@majedelass @jorytindall
Updating wording to more accurate (thanks Jory)
cd6eabd 
Co-authored-by: Jory Tindall <jory.tindall@hashicorp.com>
@majedelass
Copy link
Contributor

We have three approvals on this, are we ok with merging or do we require more updates?

@didoo
Copy link
Contributor Author

didoo commented Apr 10, 2024 •
edited
Loading

We have three approvals on this, are we ok with merging or do we require more updates?

@majedelass OK to merge, for me

Wait from what I see it's not approved in Figma, so we should wait to merge them at the same time (more or less)
screenshot_3649

@didoo @KristinLBradley
Apply suggestions from code review
3f35974 
Co-authored-by: Kristin Bradley <kristin.bradley@hashicorp.com>
@majedelass majedelass merged commit df907f2 into main Apr 11, 2024
16 checks passed
@majedelass majedelass deleted the dropdown-listitem-interactive-trailingicon branch April 11, 2024 17:37
@hashibot-hds hashibot-hds mentioned this pull request Apr 11, 2024
Merged
didoo added a commit that referenced this pull request May 9, 2024
@didoo
added missing CSS class to the `hds-dropdown-list-item__interactive-i…
9a210a1 
…con` element

(regression happened in #2042)
@didoo didoo mentioned this pull request May 9, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alex-ju alex-ju alex-ju approved these changes

@majedelass majedelass majedelass approved these changes

@jorytindall jorytindall jorytindall approved these changes

@KristinLBradley KristinLBradley KristinLBradley approved these changes

Assignees
No one assigned
Labels
docs-website Content updates to the documentation website packages/components showcase
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@didoo @majedelass @alex-ju @jorytindall @KristinLBradley @hashibot-hds