-
Notifications
You must be signed in to change notification settings - Fork 0
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
Header harmony #1213
base: main
Are you sure you want to change the base?
Header harmony #1213
Conversation
16eb20e
to
e7878bc
Compare
1c655d3
to
5cff4bf
Compare
2b18aea
to
2d4fd61
Compare
/canary |
Canary available: |
src/harmony/Header/Header.module.css
Outdated
|
||
.HeaderNavLink:hover { | ||
color: var(--base-white); | ||
border-color: var(--link0); |
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.
this prop doesn't exist in css harmony styles
src/harmony/Header/Header.module.css
Outdated
} | ||
|
||
.HeaderNavLink:active { | ||
color: var(--link0); |
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.
same
src/harmony/Link/Link.module.css
Outdated
.Link_primary { | ||
color: var(--text-primary); | ||
text-decoration-line: none; | ||
} | ||
|
||
.Link_primary:hover { | ||
color: var(--blue-500); | ||
color: var(--link0); |
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.
doesn't exist in harmony
src/harmony/Link/Link.module.css
Outdated
} | ||
|
||
.Link_inline { | ||
color: inherit; | ||
text-decoration-line: underline; | ||
font-size: var(--link0); |
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.
same
src/harmony/Link/Link.module.css
Outdated
@@ -1,19 +1,29 @@ | |||
.Link { | |||
transition: color 200ms ease-in; | |||
font-size: 22px; |
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.
why size in pixels not rems?
src/harmony/Header/Header.tsx
Outdated
|
||
export const HeaderNavLink: FC<AnchorHTMLAttributes<HTMLAnchorElement>> = ({ className, children, ...props }) => { | ||
return ( | ||
<a className={cn(s.HeaderNavLink, className)} {...props}> |
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.
You can use the Link
component instead of plain html a
element
8f74065
to
83d2219
Compare
83d2219
to
3f9b02a
Compare
/canary |
Canary available: |
PR includes
Related issues
Resolve #1212