Skip to content
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

Refresh: misc tweaks and refinements #15654

Merged
merged 6 commits into from
Dec 4, 2024
Merged

Refresh: misc tweaks and refinements #15654

merged 6 commits into from
Dec 4, 2024

Conversation

craigcook
Copy link
Member

  • Adjust font sizes on some Firefox pages [Refresh: Pre-launch checklist #15416] - XL in the new type scale is quite large indeed, so some pages with big titles got TOO big and needed to be taken down a notch.
  • Use Mozilla Text in the footer for the donate link, language picker, and newsletter button (we shouldn't use the headline font for anything smaller than 24px).
  • Update the heart icon in the footer donate button. Also simplified some related CSS.
  • Some minor cleanup and refactoring of language picker styles.
  • Made the language link visible on focus (I was confused when tabbing through and it took me a while to realize the link was there).
  • Gave the social icons a slightly bigger hit area.

@craigcook craigcook added P2 Second level priority - Should have Needs Review Awaiting code review Review: XS Code review time: up to 30min Refresh 🦖 Work related to the 2024 site refresh labels Dec 4, 2024
@craigcook craigcook requested a review from a team as a code owner December 4, 2024 01:19
@stephaniehobson stephaniehobson self-assigned this Dec 4, 2024
stephaniehobson
stephaniehobson requested changes Dec 4, 2024
View reviewed changes
Copy link
Contributor

@stephaniehobson stephaniehobson left a comment

Choose a reason for hiding this comment

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

Lots of good improvements 🔥 Two small unintended side effects to fix.

(not blocking) The language selector is still in Headline. I only mention it because you said you tweaked it.

media/css/m24/components/footer-refresh.scss Show resolved Hide resolved
media/css/m24/components/footer-refresh.scss Show resolved Hide resolved
media/css/m24/components/footer-refresh.scss Show resolved Hide resolved
This was referenced Dec 4, 2024
Closed
Open
Closed
@janbrasna janbrasna mentioned this pull request Dec 4, 2024
Merged
@craigcook craigcook force-pushed the m24-misc branch 2 times, most recently from ea49349 to 033533f Compare December 4, 2024 18:18
@codecov Codecov
Copy link

codecov bot commented Dec 4, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.04%. Comparing base (d645d22) to head (82ca196).
Report is 3 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #15654   +/-   ##
=======================================
  Coverage   79.04%   79.04%           
=======================================
  Files         159      159           
  Lines        8329     8329           
=======================================
  Hits         6584     6584           
  Misses       1745     1745

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@craigcook
Copy link
Member Author

I'm seeing the language picker in Text, not Headline... maybe local cache?

@craigcook
Refresh: misc tweaks and refinements
2633c9b 
* Adjust font sizes on some Firefox pages [mozilla#15416]
* Update fonts and icons in some footer elements.
* Some minor refactoring and tidying in footer.
@craigcook
review fixes, add caret icons
0be5700
@craigcook
Clean up footer media queries [fix mozilla#15668]
6eddfba
@craigcook craigcook mentioned this pull request Dec 4, 2024
Closed
janbrasna
janbrasna reviewed Dec 4, 2024
View reviewed changes
media/css/m24/components/footer-refresh.scss
@media(min-width: $screen-lg) {
@media #{$mq-lg} {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for these ❤️ — The same is in footer-newsletter if you fancy taking a look too.

janbrasna
janbrasna reviewed Dec 4, 2024
View reviewed changes
l10n/en/navigation_refresh.ftl Outdated
Comment on lines 25 to 26
navigation-refresh-mozilla-ai-v2 = { -brand-name-mozilla-ai-v2 }
navigation-refresh-mozilla-ventures = { -brand-name-mozilla } Ventures
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this may actually need:

Suggested change
navigation-refresh-mozilla-ai-v2 = { -brand-name-mozilla-ai-v2 }
navigation-refresh-mozilla-ventures = { -brand-name-mozilla } Ventures
# Obsolete string (expires: 2025-01-05)
navigation-refresh-mozilla-ai-v2 = { -brand-name-mozilla-ai }
navigation-refresh-mozilla-ai-v3 = { -brand-name-mozilla-ai-v2 }
# Obsolete string (expires: 2025-01-05)
navigation-refresh-mozilla-ventures = { -brand-name-mozilla } Ventures

so that would mean navigation-refresh-mozilla-ai-v3 to the submenu html?

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm true... what a mess 😄

@janbrasna janbrasna mentioned this pull request Dec 4, 2024
Merged
stephaniehobson
stephaniehobson approved these changes Dec 4, 2024
View reviewed changes
Copy link
Contributor

@stephaniehobson stephaniehobson left a comment

Choose a reason for hiding this comment

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

R+ 🦠

@craigcook craigcook merged commit d9f20f8 into mozilla:main Dec 4, 2024
6 checks passed
@craigcook craigcook deleted the m24-misc branch December 4, 2024 22:21
@craigcook craigcook mentioned this pull request Dec 4, 2024
Merged
@wen-2018 wen-2018 mentioned this pull request Dec 5, 2024
Closed
janbrasna
janbrasna reviewed Dec 10, 2024
View reviewed changes
media/img/icons/m24-small/up-caret.svg
Copy link
Contributor

Choose a reason for hiding this comment

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

The up-caret Is white instead of black. Is that intentional, or should be changes to the expected #000?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, a save-as error that went unnoticed because we're not actually using the black version anywhere (yet). Sharp eye!

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 it's getting fixed in #15704

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janbrasna janbrasna janbrasna left review comments

@stephaniehobson stephaniehobson stephaniehobson approved these changes

Assignees

@stephaniehobson stephaniehobson

Labels
Needs Review Awaiting code review P2 Second level priority - Should have Refresh 🦖 Work related to the 2024 site refresh Review: XS Code review time: up to 30min
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@craigcook @stephaniehobson @janbrasna