Closed JoshBeveridge closed 10 months ago
@substrae Could you please fix the Figma link? Also, are there any differences in mobile view?
@petertgiles
Done, and I don't think anything changes layout-wise, but the information changes should be mirrored.
@substrae Right now the icon is set to the size of the font of the heading, which can change. Do you want to fix the icon size regardless of the font size or grow the icons with the heading levels?
@petertgiles
Hmm, there isn't any instance where this component's heading size will change, so perhaps just remove that functionality?
@esizer Looks like the size is indeed fixed, though the heading level can change. https://github.com/GCTC-NTGC/gc-digital-talent/blob/9909cdf56694c1c7325a76ee1fcbf5ad17221cc0/apps/web/src/components/ExperienceCard/ExperienceCard.tsx#L171
@substrae Looks like the icon is currently 1.2em. I'm not sure if x1 is definitely larger? https://github.com/GCTC-NTGC/gc-digital-talent/blob/9909cdf56694c1c7325a76ee1fcbf5ad17221cc0/apps/web/src/components/ExperienceCard/ExperienceCard.tsx#L144
@petertgiles
Because em
is relative to the last font-size
definition declared in the tree, it very well might be smaller than x1
in this instance, which is always a fixed value based on the html
's base font-size
.
Hmm, there isn't any instance where this component's heading size will change, so perhaps just remove that functionality?
Our headings determine the icon size based on the font size. We did this so we don't need to specify size every time we put an icon next to a heading and to keep things consistent.
What if, instead of relying on em
we use a the map that defines font-size?
export const headingStyles: Record<HeadingLevel, Record<string, string>> = {
h1: {
"data-h2-font-size": "base(h1)",
"data-h2-height": "base:children[svg](x2)"
"data-h2-width": "base:children[svg](x2)"
},
...
}
We would just need to know the size value of the icon for each heading size.
@petertgiles @esizer
Oh awesome - a map makes sense to me. This has actually already been done in Figma too. Check them out in the heading component: https://www.figma.com/file/guHeIIh8dqFVCks310Wv0G/Style-library?type=design&node-id=9-31&mode=design
Nice! Could you please add the map to this issue? I can only find pixels in Figma.
@petertgiles
You bet!
h6 > x.8
h5 > x.95
h4 > x1.15
h3 > x1.35
h2 > x1.65
h1 > x2
display > x2.35
@substrae Was it intentional that the icon got darker? This colour header icon is used in a few other places, like the Directive section on the executive home page and will be affected by this change.
@petertgiles
Hmm, I was always under the impression they were tertiary.darker
(they were next to text of the same color in the previous layout, which required that value for contrast). Do you have examples of how/what this changes in other contexts?
Do you have examples of how/what this changes in other contexts?
Eric's PR has a Chromatic diff but he hasn't actually changed the colour of the icon yet. Here's a few spots where a tertiary colour header icon is used though.
@petertgiles
Ahh, hmm. I suppose we can keep this as is (tertiary
) now that it's not next to colored text. I'll update the Figma file.
✨ Summary
Visit the Figma file • Chat with @substrae for design questions
We've received two minor pieces of feedback from user testing:
This ticket focuses primarily on moving the dates around, however the icon has also been emphasized as a first pass at helping with type identification. More changes will follow in a separate ticket when a better solution has been found.
🔍 Details
Below are screenshots that showcase how the details section should now respond layout-wise with the absence of the date information. It's also worth noting that because the
award
type only uses a single date, its subline appears slightly different than the other types.📸 Work experience details
📸 Education and certificate details
📸 Community participation details
📸 Personal learning details
📸 Awards and recognition details