fix(navigation): final minor fixes

[giamir]

SPARK-130

Top margin of section title is 16px in Stacks classic, design says this should be 24px.

Chevron and any other icons in the section header should be black-400

Those 2 points have been addressed directly in stacks classic. The rest are all updates of the storybook trailing story to show one way of achieving the desired final spec. Consumer can take heavy inspiration from that story but they will ultimately have to replicate something simlar for their concrete use case.

Chevron icon used should be the default size (20x20) — we don’t have all the 16x16 sizes for components like compose and we’ll need to add that compose icon later for editing the communities list.

Override the button small padding to use p6 (for chevron and other button icons like compose)

Addressed in the storybook trailing story.

Chevron rotation: I’m assuming the devs chose to rotate IconChevronUp instead of using IconChevronDown to create the animation of it twirling

I kept this as it was in the trailing story. I personally think the added complexity is not worth it. That said if @kylejrp wants to use his approach with the extra fade effect he can by all mean. The example is just an example and can be tweaked depending on the use case. There is no plan of adding those animation directly in the system libraries for now.

Get rid of the extra margin space when a section header is collapsed

Addressed in the trailing story with a conditional mn24 class being applied. Again this is something that the consumers of Stacks can decide how to best handle.

Include example of the editable icon or collapsed activity icon alongside the expandable icon

This have been added to the trailing story to show how it can be achieved by consumers of Stacks.

Footer

Out of scope, it should certainly be a separate navigation (nav) landmark. I think for the time being consumers can implement that using the Link components. If we feel the need to standardize the footer we should extract it as we have done in the past for the topbar but that's a totally different story.

Trailing Story

Extra changes:

Hover state should be black-100 (different than the selected state)

This has been addressed. It is a stacks classic change.

Is it possible to set a min height to the section title container with the label? Because we're not matching the icon size anymore (because it's in a button now) we loose some of the height because the label text is only ~15px. Can we make this be closer to a min height of 20px?

A similar effect has been achieved by making the padding vertical of s-navigation--title elements 18 instead of 16.

[changeset-bot]

🦋 Changeset detected

Latest commit: 524c1f8

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for stacks ready!

Name	Link
🔨 Latest commit	524c1f8
🔍 Latest deploy log	https://app.netlify.com/projects/stacks/deploys/692dc03543096b0009954c13
😎 Deploy Preview	https://deploy-preview-2081--stacks.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[netlify]

✅ Deploy Preview for stacks-svelte ready!

Name	Link
🔨 Latest commit	524c1f8
🔍 Latest deploy log	https://app.netlify.com/projects/stacks-svelte/deploys/692dc035722c5d0008199700
😎 Deploy Preview	https://deploy-preview-2081--stacks-svelte.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[CGuindon]

These changes LGTM — thanks for tackling those so quickly! Sorry I didn't catch the icon sizing on the first round.

Wondering if we should let @kylejrp review as well? I've sent the preview to @ellenchanhy to have a quick look to and double check but I think this works for what we need.

[CGuindon]

Oops sorry — missed something.

• Hover state should be black-100 (different than the selected state.
• Is it possible to set a min height to the section title container with the label? Because we're not matching the icon size anymore (because it's in a button now) we loose some of the height because the label text is only ~15px. Can we make this be closer to a min height of 20px?

Note I looked at swapping the padding 16 to 24 but that's too big

[giamir]

Is it possible to set a min height to the section title container with the label? Because we're not matching the icon size anymore (because it's in a button now) we loose some of the height because the label text is only ~15px. Can we make this be closer to a min height of 20px?

@CGuindon I am not sure if I totally understand this. The height of the s-navigation--title container includes also the padding top and bottom which are currently 16. Setting a min height of 20px to that container won't have any effect.
I believe we probably can achieve a similar effect to what I think you want by bumping the padding top and bottom of the title container from 16 to 18.

Shall I do that?

[CGuindon]

@giamir Confirming, yes let's do your suggestion of making the padding 18px

[CGuindon]

🙌