-
Notifications
You must be signed in to change notification settings - Fork 44
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
Dropdown component, list items #68
Dropdown component, list items #68
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). hds-flight-website – ./🔍 Inspect: https://vercel.com/hashicorp/hds-flight-website/4neCWjm6xviWSvcWuH36DbjDqanx hds-components – ./🔍 Inspect: https://vercel.com/hashicorp/hds-components/4uCU1F4haFZSqab1G6VhGQsreDR6 |
@didoo I refactored the link color class name (you were right, I had them done incorrectly). I also wrapped the link text and icon in their own divs so we could control icon placement when there is a longer item whose text wraps (see photos). |
ok @didoo I think this one is ready to roll up into the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MelSumner I have added a bunch of comments. the most critical one is that some merge/conflict was not fixed in the right way and a bunch of code have been lost (the styling of the links for action/critical
with all the states)
export const DEFAULT_COLOR = 'action'; | ||
export const DEFAULT_ITEM = 'link'; | ||
export const COLORS = ['action', 'critical']; | ||
export const ITEMS = ['heading', 'help-text', 'separator', 'copy-item', 'link']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[non-blocker] I am not sure we're going to implement the copy-item
in the first release (see comments in #84) so if you want to remove any reference to it in this PR or in the feature branch, is totally OK for me
packages/components/addon/components/hds/dropdown/toggle-overflow.js
Outdated
Show resolved
Hide resolved
packages/components/addon/components/hds/dropdown/toggle-user.hbs
Outdated
Show resolved
Hide resolved
packages/components/addon/components/hds/dropdown/toggle-user.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pair programmed with @MelSumner and fixed all the issues together
📌 Summary
If merged, this PR would add the dropdown items to the dropdown component branch.
📸 Screenshots
Current state:
🔗 External links
✅ Reviewer's checklist
💬 Please consider using conventional comments when reviewing this PR.