Skip to content

Scale down logo more for small screens #1517

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Aug 31, 2021
Merged

Scale down logo more for small screens #1517

merged 5 commits into from
Aug 31, 2021

Conversation

calebeby
Copy link
Member

@calebeby calebeby commented Aug 31, 2021 •
edited
Loading

@changeset-bot
Copy link

changeset-bot bot commented Aug 31, 2021 •
edited
Loading

🦋 Changeset detected

Latest commit: 6918af2

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

This PR includes changesets to release 1 package
Name Type
@cloudfour/patterns Minor

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
Copy link

netlify bot commented Aug 31, 2021 •
edited
Loading

✔️ Deploy Preview for cloudfour-patterns ready!

🔨 Explore the source changes: 6918af2

🔍 Inspect the deploy log: https://app.netlify.com/sites/cloudfour-patterns/deploys/612e5d9196a5930007501012

😎 Browse the preview: https://deploy-preview-1517--cloudfour-patterns.netlify.app

@calebeby calebeby marked this pull request as ready for review August 31, 2021 16:19
@calebeby calebeby requested a review from tylersticka August 31, 2021 16:20
calebeby
calebeby commented Aug 31, 2021
View reviewed changes
src/components/sky-nav/sky-nav.scss Outdated
transition: opacity transition.$slow ease.$out,
transform transition.$quick ease.$out;
width: ms.step(8);
width: fluid.fluid-calc(breakpoint.$s, breakpoint.$l, ms.step(7), ms.step(9));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tylersticka Any idea why the last number has to be 9? If I change it to 8, at the large breakpoint it is very discontinuous and there is a large jump. I tried to figure out what was going wrong but I couldn't figure it out. It might have something to do with the units?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm honestly not exactly sure, the math I stole from CSS-Tricks articles. I've been considering whether long-term we should replace this logic with something simpler based on math functions like min and max, which I didn't realize until recently are startlingly well-supported.

Can you add a comment explaining why it's set to 9 here?

tylersticka
tylersticka reviewed Aug 31, 2021
View reviewed changes
Copy link
Member

@tylersticka tylersticka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels so much better. Will approve once we add a Sass comment so future generations understand why we used ms.step(9) (always good to explain magic numbers, y'know?), but no issues at all with the end result.

tylersticka
tylersticka previously approved these changes Aug 31, 2021
View reviewed changes
@calebeby calebeby requested a review from tylersticka August 31, 2021 16:49
@calebeby calebeby merged commit 09079e1 into v-next Aug 31, 2021
@calebeby calebeby deleted the smaller-logo-small-screens branch August 31, 2021 16:52
@github-actions github-actions bot mentioned this pull request Aug 31, 2021
Merged
gerardo-rodriguez added a commit that referenced this pull request Sep 2, 2021
@gerardo-rodriguez
Merge branch 'v-next' of github.com:cloudfour/cloudfour.com-patterns …
961a6d6 
…into feature/button-swap

* 'v-next' of github.com:cloudfour/cloudfour.com-patterns:
  Introduce Button 'is-slashed' CSS state class, remove aria-pressed logic (#1519)
  Scale down logo more for small screens (#1517)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tylersticka tylersticka tylersticka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@calebeby @tylersticka