Skip to content

Conversation

@dmarcey
Copy link
Contributor

@dmarcey dmarcey commented Sep 30, 2019

Updated Link to accept a muted prop to achieve the styles found here: https://primer.style/css/utilities/colors#link-colors
Closes #561

@vercel
Copy link

vercel bot commented Sep 30, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://primer-components-git-muted-link.primer.now.sh

@dmarcey dmarcey requested a review from emplums September 30, 2019 17:15
@@ -1,4 +1,3 @@
import loadable from '@loadable/component'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes a lint warning

appearance: 'none'
}

const hoverColor = system({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure what this was doing. I couldn't find any color emitted for :hover on the links. It seems like for normal links the color is the same on hover, but if this needs to be added back for another reason, let me know!

Copy link

Choose a reason for hiding this comment

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

This adds a custom styled system prop to the component so that users of the component can configure what color the links should be on hover. I think we should put this back in - there were a few places where we were changing the hover color intentionally - I believe the primer.style site still uses it!

Copy link

@emplums emplums left a comment

Choose a reason for hiding this comment

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

Just made one suggestion otherwise this looks good!

Co-Authored-By: Emily <emplums@github.com>
@vercel vercel bot temporarily deployed to staging September 30, 2019 17:28 Inactive
@vercel vercel bot temporarily deployed to staging September 30, 2019 17:33 Inactive
@vercel vercel bot temporarily deployed to staging September 30, 2019 17:52 Inactive
@dmarcey dmarcey requested a review from emplums September 30, 2019 18:56
Copy link

@emplums emplums left a comment

Choose a reason for hiding this comment

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

This is good to go! If you can just bump the version we can merge & release :)

@vercel vercel bot temporarily deployed to staging October 1, 2019 16:53 Inactive
@vercel vercel bot temporarily deployed to staging October 1, 2019 17:03 Inactive
@dmarcey dmarcey merged commit ad80cdd into master Oct 1, 2019
@emplums emplums deleted the muted-link branch October 22, 2019 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

✏️[Update] Muted Link

3 participants