Skip to content
This repository was archived by the owner on Mar 8, 2023. It is now read-only.

feat(link): add icon-placement option#1009

Open
emyarod wants to merge 4 commits intocarbon-design-system:mainfrom
emyarod:feat/link-icon-placement
Open

feat(link): add icon-placement option#1009
emyarod wants to merge 4 commits intocarbon-design-system:mainfrom
emyarod:feat/link-icon-placement

Conversation

@emyarod
Copy link
Copy Markdown
Member

@emyarod emyarod commented Jun 14, 2022

Related Ticket(s)

carbon-design-system/carbon-for-ibm-dotcom#8836

Description

This PR adds support for icon-placement in the link component to potentially deprecate the link-with-icon dotcom component

@emyarod emyarod requested a review from a team as a code owner June 14, 2022 16:58
@emyarod emyarod requested review from RichKummer and annawen1 and removed request for a team June 14, 2022 16:58
@ibmdotcom-bot
Copy link
Copy Markdown
Contributor

ibmdotcom-bot commented Jun 14, 2022

@ibmdotcom-bot
Copy link
Copy Markdown
Contributor

ibmdotcom-bot commented Jun 14, 2022

@ibmdotcom-bot
Copy link
Copy Markdown
Contributor

ibmdotcom-bot commented Jun 14, 2022

@ibmdotcom-bot
Copy link
Copy Markdown
Contributor

ibmdotcom-bot commented Jun 14, 2022

Copy link
Copy Markdown
Member

@ariellalgilmore ariellalgilmore left a comment

Choose a reason for hiding this comment

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

LGTM! If possible can we just move the iconPlacement knob to just the pairedWithIcon story

@RichKummer
Copy link
Copy Markdown

Agree with @ariellalgilmore on the knob update! I noticed that there seems to be some extra padding on the left when the icon is positioned on the left:

Positioned right
Screen Shot 2022-06-17 at 10 59 42 AM

Positioned left
Screen Shot 2022-06-17 at 10 59 28 AM

Not sure if this would cause downstream effects. Also Percy's picking up 72 differences, but it looks like these may just be new pages or Percy's creating this for the first time. Figured I would flag just in case:
Screen Shot 2022-06-17 at 10 54 51 AM

@emyarod
Copy link
Copy Markdown
Member Author

emyarod commented Jun 17, 2022

@ariellalgilmore @RichKummer the knob should be on the Paired with icon story only now! also fixed the extra spacing when the icon is placed on the left side. and for the Percy changes, it looks like the snapshots are all brand new

Copy link
Copy Markdown

@RichKummer RichKummer left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for fixing @emyarod !

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants