-
Notifications
You must be signed in to change notification settings - Fork 645
chore(Link): Remove the CSS modules feature flag from the Link component #5148
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
Changes from all commits
92c1fad
8b31cf9
3e6c0f1
b6b8447
2c598a2
8dfeec6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@primer/react": minor | ||
| --- | ||
|
|
||
| Remove the CSS modules feature flag from the Link component |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,14 @@ | ||
| import {clsx} from 'clsx' | ||
| import React, {forwardRef, useEffect} from 'react' | ||
| import styled from 'styled-components' | ||
| import {system} from 'styled-system' | ||
| import {get} from '../constants' | ||
| import {useRefObjectAsForwardedRef} from '../hooks' | ||
| import type {SxProp} from '../sx' | ||
| import sx from '../sx' | ||
| import classes from './Link.module.css' | ||
| import {useFeatureFlag} from '../FeatureFlags' | ||
| import Box from '../Box' | ||
| import type {ComponentProps} from '../utils/types' | ||
| import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' | ||
|
|
||
| type StyledLinkProps = { | ||
| /** @deprecated use CSS modules to style hover color */ | ||
| hoverColor?: string | ||
| muted?: boolean | ||
| /** @deprecated use `inline` to specify the type of link instead */ | ||
|
|
@@ -21,49 +17,7 @@ type StyledLinkProps = { | |
| inline?: boolean | ||
| } & SxProp | ||
|
|
||
| const hoverColor = system({ | ||
| hoverColor: { | ||
| property: 'color', | ||
| scale: 'colors', | ||
| }, | ||
| }) | ||
|
|
||
| const StyledLink = styled.a<StyledLinkProps>` | ||
| color: ${props => (props.muted ? get('colors.fg.muted')(props) : get('colors.accent.fg')(props))}; | ||
|
|
||
| /* By default, Link does not have underline */ | ||
| text-decoration: none; | ||
|
|
||
| /* You can add one by setting underline={true} */ | ||
| text-decoration: ${props => (props.underline ? 'underline' : undefined)}; | ||
|
|
||
| /* Inline links (inside a text block), however, should have underline based on accessibility setting set in data-attribute */ | ||
| /* Note: setting underline={false} does not override this */ | ||
| [data-a11y-link-underlines='true'] &[data-inline='true'] { | ||
| text-decoration: underline; | ||
| } | ||
|
|
||
| &:hover { | ||
| text-decoration: ${props => (props.muted ? 'none' : 'underline')}; | ||
| ${props => (props.hoverColor ? hoverColor : props.muted ? `color: ${get('colors.accent.fg')(props)}` : '')}; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we passing along the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not doing anything anymore |
||
| } | ||
| &:is(button) { | ||
| display: inline-block; | ||
| padding: 0; | ||
| font-size: inherit; | ||
| white-space: nowrap; | ||
| cursor: pointer; | ||
| user-select: none; | ||
| background-color: transparent; | ||
| border: 0; | ||
| appearance: none; | ||
| } | ||
| ${sx}; | ||
| ` | ||
|
|
||
| const Link = forwardRef(({as: Component = 'a', className, inline, underline, ...props}, forwardedRef) => { | ||
| const enabled = useFeatureFlag('primer_react_css_modules_ga') | ||
|
|
||
| const innerRef = React.useRef<HTMLAnchorElement>(null) | ||
| useRefObjectAsForwardedRef(forwardedRef, innerRef) | ||
|
|
||
|
|
@@ -91,24 +45,10 @@ const Link = forwardRef(({as: Component = 'a', className, inline, underline, ... | |
| }, [innerRef]) | ||
| } | ||
|
|
||
| if (enabled) { | ||
| if (props.sx) { | ||
| return ( | ||
| <Box | ||
| as={Component} | ||
| className={clsx(className, classes.Link)} | ||
| data-muted={props.muted} | ||
| data-inline={inline} | ||
| data-underline={underline} | ||
| {...props} | ||
| // @ts-ignore shh | ||
| ref={innerRef} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| if (props.sx) { | ||
| return ( | ||
| <Component | ||
| <Box | ||
| as={Component} | ||
| className={clsx(className, classes.Link)} | ||
| data-muted={props.muted} | ||
| data-inline={inline} | ||
|
|
@@ -121,11 +61,11 @@ const Link = forwardRef(({as: Component = 'a', className, inline, underline, ... | |
| } | ||
|
|
||
| return ( | ||
| <StyledLink | ||
| as={Component} | ||
| className={className} | ||
| <Component | ||
| className={clsx(className, classes.Link)} | ||
| data-muted={props.muted} | ||
| data-inline={inline} | ||
| underline={underline} | ||
| data-underline={underline} | ||
| {...props} | ||
| // @ts-ignore shh | ||
| ref={innerRef} | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I probably don't need this ref anymore? @joshblack
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's used in the effect for checking the ref type IIRC, let me know if I'm missing something 👀 |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,203 +1,41 @@ | ||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
|
||
| exports[`Link applies button styles when rendering a button element 1`] = ` | ||
| .c1 { | ||
| color: var(--fgColor-accent,var(--color-accent-fg,#0969da)); | ||
| -webkit-text-decoration: none; | ||
| text-decoration: none; | ||
| } | ||
| [data-a11y-link-underlines='true'] .c0[data-inline='true'] { | ||
| -webkit-text-decoration: underline; | ||
| text-decoration: underline; | ||
| } | ||
| .c1:hover { | ||
| -webkit-text-decoration: underline; | ||
| text-decoration: underline; | ||
| } | ||
| .c1:is(button) { | ||
| display: inline-block; | ||
| padding: 0; | ||
| font-size: inherit; | ||
| white-space: nowrap; | ||
| cursor: pointer; | ||
| -webkit-user-select: none; | ||
| -moz-user-select: none; | ||
| -ms-user-select: none; | ||
| user-select: none; | ||
| background-color: transparent; | ||
| border: 0; | ||
| -webkit-appearance: none; | ||
| -moz-appearance: none; | ||
| appearance: none; | ||
| } | ||
| <button | ||
| className="c0 c1" | ||
| className="Link" | ||
| /> | ||
| `; | ||
|
|
||
| exports[`Link passes href down to link element 1`] = ` | ||
| .c1 { | ||
| color: var(--fgColor-accent,var(--color-accent-fg,#0969da)); | ||
| -webkit-text-decoration: none; | ||
| text-decoration: none; | ||
| } | ||
| [data-a11y-link-underlines='true'] .c0[data-inline='true'] { | ||
| -webkit-text-decoration: underline; | ||
| text-decoration: underline; | ||
| } | ||
| .c1:hover { | ||
| -webkit-text-decoration: underline; | ||
| text-decoration: underline; | ||
| } | ||
| .c1:is(button) { | ||
| display: inline-block; | ||
| padding: 0; | ||
| font-size: inherit; | ||
| white-space: nowrap; | ||
| cursor: pointer; | ||
| -webkit-user-select: none; | ||
| -moz-user-select: none; | ||
| -ms-user-select: none; | ||
| user-select: none; | ||
| background-color: transparent; | ||
| border: 0; | ||
| -webkit-appearance: none; | ||
| -moz-appearance: none; | ||
| appearance: none; | ||
| } | ||
| <a | ||
| className="c0 c1" | ||
| className="Link" | ||
| href="https://github.com" | ||
| /> | ||
| `; | ||
|
|
||
| exports[`Link respects hoverColor prop 1`] = ` | ||
| .c1 { | ||
| color: var(--fgColor-accent,var(--color-accent-fg,#0969da)); | ||
| -webkit-text-decoration: none; | ||
| text-decoration: none; | ||
| } | ||
| [data-a11y-link-underlines='true'] .c0[data-inline='true'] { | ||
| -webkit-text-decoration: underline; | ||
| text-decoration: underline; | ||
| } | ||
| .c1:hover { | ||
| -webkit-text-decoration: underline; | ||
| text-decoration: underline; | ||
| color: var(--fgColor-accent,var(--color-accent-fg,#0969da)); | ||
| } | ||
| .c1:is(button) { | ||
| display: inline-block; | ||
| padding: 0; | ||
| font-size: inherit; | ||
| white-space: nowrap; | ||
| cursor: pointer; | ||
| -webkit-user-select: none; | ||
| -moz-user-select: none; | ||
| -ms-user-select: none; | ||
| user-select: none; | ||
| background-color: transparent; | ||
| border: 0; | ||
| -webkit-appearance: none; | ||
| -moz-appearance: none; | ||
| appearance: none; | ||
| } | ||
| <a | ||
| className="c0 c1" | ||
| className="Link" | ||
| hoverColor="accent.fg" | ||
| /> | ||
| `; | ||
|
|
||
| exports[`Link respects the "sx" prop when "muted" prop is also passed 1`] = ` | ||
| .c1 { | ||
| color: var(--fgColor-muted,var(--color-fg-muted,#656d76)); | ||
| -webkit-text-decoration: none; | ||
| text-decoration: none; | ||
| .c0 { | ||
| color: var(--fgColor-onEmphasis,var(--color-fg-on-emphasis,#ffffff)); | ||
| } | ||
| [data-a11y-link-underlines='true'] .c0[data-inline='true'] { | ||
| -webkit-text-decoration: underline; | ||
| text-decoration: underline; | ||
| } | ||
| .c1:hover { | ||
| -webkit-text-decoration: none; | ||
| text-decoration: none; | ||
| color: var(--fgColor-accent,var(--color-accent-fg,#0969da)); | ||
| } | ||
| .c1:is(button) { | ||
| display: inline-block; | ||
| padding: 0; | ||
| font-size: inherit; | ||
| white-space: nowrap; | ||
| cursor: pointer; | ||
| -webkit-user-select: none; | ||
| -moz-user-select: none; | ||
| -ms-user-select: none; | ||
| user-select: none; | ||
| background-color: transparent; | ||
| border: 0; | ||
| -webkit-appearance: none; | ||
| -moz-appearance: none; | ||
| appearance: none; | ||
| } | ||
| <a | ||
| className="c0 c1" | ||
| className="c0 Link" | ||
| data-muted={true} | ||
| muted={true} | ||
| /> | ||
| `; | ||
|
|
||
| exports[`Link respects the "muted" prop 1`] = ` | ||
| .c1 { | ||
| color: var(--fgColor-muted,var(--color-fg-muted,#656d76)); | ||
| -webkit-text-decoration: none; | ||
| text-decoration: none; | ||
| } | ||
| [data-a11y-link-underlines='true'] .c0[data-inline='true'] { | ||
| -webkit-text-decoration: underline; | ||
| text-decoration: underline; | ||
| } | ||
| .c1:hover { | ||
| -webkit-text-decoration: none; | ||
| text-decoration: none; | ||
| color: var(--fgColor-accent,var(--color-accent-fg,#0969da)); | ||
| } | ||
| .c1:is(button) { | ||
| display: inline-block; | ||
| padding: 0; | ||
| font-size: inherit; | ||
| white-space: nowrap; | ||
| cursor: pointer; | ||
| -webkit-user-select: none; | ||
| -moz-user-select: none; | ||
| -ms-user-select: none; | ||
| user-select: none; | ||
| background-color: transparent; | ||
| border: 0; | ||
| -webkit-appearance: none; | ||
| -moz-appearance: none; | ||
| appearance: none; | ||
| } | ||
| <a | ||
| className="c0 c1" | ||
| className="Link" | ||
| data-muted={true} | ||
| muted={true} | ||
| /> | ||
| `; |
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.
If you haven't already, could you add a note to https://github.com/github/primer/issues/3541 for removing this from
Link? 🙏