-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: update projects page #51
Conversation
chosww
commented
Jan 8, 2025
•
edited
Loading
edited
- Update projects page
- Implement project panel component for each project
- Create singular project page
- Implement interaction styles for the card component
Deploying inclusive-standards with Cloudflare Pages
|
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.
A few more changes. I also note you haven't added interactive styles for links. The issue which you've indicated that this PR closes lists buttons, selects, and accordions but I don't see those here… perhaps better to remove that from the scope of this PR as we don't have all those elements yet?
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.
Only once change left here— replace all hex colours within the style declarations with variables so if we need to change them we can do it in one place!
@@ -2,34 +2,34 @@ | |||
color: black; | |||
min-width: 20.5rem; | |||
margin-bottom: 1rem; | |||
box-shadow: 0 0.625rem 1.25rem 0 #0B046C26; | |||
box-shadow: | |||
0 0.625rem 1.25rem 0 #0B046C26, |
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.
Ideally we should always use a colour variable.
.card:has(a:focus) { | ||
border: 0.1875rem solid transparent; | ||
box-shadow: 0 0.625rem 1.25rem 0 #0B046C26; |
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.
Use a colour variable here.
.card:hover { | ||
box-shadow: | ||
inset 0 0 0 0.1875rem var(--fl-fgColor, transparent), | ||
0 0.625rem 1.25rem 0 var(--fl-fgColor, #0B046C73); |
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.
Use a colour variable here.
min-height: 2.5rem; | ||
.card:hover.card:has(a:focus) { | ||
box-shadow: | ||
0 0.625rem 1.25rem 0 var(--fl-fgColor, #0B046C73); |
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.
Use a colour variable here.
position: relative; | ||
} | ||
|
||
.project-panel:has(a:focus) { | ||
border: 0.1875rem solid transparent; | ||
box-shadow: 0 0.625rem 1.25rem 0 #0B046C26; |
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.
Colour variable here.
box-shadow: 0 0.625rem 1.25rem 0 var(--fl-fgColor, #0B046C73); | ||
box-shadow: | ||
inset 0 0 0 0.1875rem var(--fl-fgColor, transparent), | ||
0 0.625rem 1.25rem 0 var(--fl-fgColor, #0B046C73); |
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.
Use a colour variable here.
box-sizing: content-box; | ||
.project-panel:hover.project-panel:has(a:focus) { | ||
box-shadow: | ||
0 0.625rem 1.25rem 0 var(--fl-fgColor, #0B046C73); |
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.
Use a colour variable here.
Also @chosww can you clarify this about the interactive styles? |
Signed-off-by: Ned Zimmerman <[email protected]>