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.

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

Simplify WNP templates #14936

Merged
merged 5 commits into from
Oct 9, 2024
Merged

Simplify WNP templates #14936

merged 5 commits into from
Oct 9, 2024

Conversation

janbrasna
Copy link
Contributor

@janbrasna janbrasna commented Jul 31, 2024

One-line summary

Moves the boilerplate ceremony to just base template, making sure the default metas are always used at least as a fallback.

Significant changes and points to review

For consistency/simplicity, this also changes:

  • all FXDE WNPs and firstrun have navbar (site_header) removed now, too;
  • adds meta description to nightly, repurposing one of the content headings;
  • deletes old font variants from WNP114 that's been removed;
  • links current (short) fx logos from img/logos where they're now available.

Issue / Bugzilla link

Resolves #14949
Refs #14720 (comment)
Refs #14822 (comment)

Testing

http://localhost:8000/en-US/firefox/131.0/whatsnew/?branch=experiment-wnp-131-tabs&variant=v1
http://localhost:8000/en-CA/firefox/131.0/whatsnew/?branch=experiment-wnp-131-tabs&variant=v2
http://localhost:8000/en-GB/firefox/131.0/whatsnew/?branch=experiment-wnp-131-tabs&variant=v3 http://localhost:8000/de/firefox/131.0/whatsnew/?branch=experiment-wnp-131-tabs&variant=v4
http://localhost:8000/es-ES/firefox/130.0/whatsnew/
http://localhost:8000/en-CA/firefox/129.0/whatsnew/
http://localhost:8000/en-GB/firefox/129.0/whatsnew/
http://localhost:8000/en-US/firefox/128.0/whatsnew/?branch=experiment-wnp-128-na-pip&variant=v1
http://localhost:8000/en-US/firefox/128.0/whatsnew/
http://localhost:8000/pl/firefox/128.0/whatsnew/?v=1
http://localhost:8000/de/firefox/128.0/whatsnew/?v=2
http://localhost:8000/fr/firefox/128.0/whatsnew/?v=3
http://localhost:8000/en-CA/firefox/127.0/whatsnew/
http://localhost:8000/it/firefox/127.0/whatsnew/
http://localhost:8000/en-US/firefox/126.0/whatsnew/
http://localhost:8000/it/firefox/126.0/whatsnew/
http://localhost:8000/en-CA/firefox/126.0beta/whatsnew/
http://localhost:8000/de/firefox/126.0beta/whatsnew/
http://localhost:8000/en-US/firefox/125.0/whatsnew/
http://localhost:8000/es-ES/firefox/125.0/whatsnew/
http://localhost:8000/firefox/66.6/whatsnew/
http://localhost:8000/en-US/firefox/99.0a1/whatsnew/
http://localhost:8000/en-US/firefox/102.0a2/whatsnew/
http://localhost:8000/en-US/firefox/92.0a2/whatsnew/
http://localhost:8000/en-US/firefox/83.0a2/firstrun/

Copy link

codecov bot commented Jul 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.88%. Comparing base (2179c6e) to head (a0225e7).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14936   +/-   ##
=======================================
  Coverage   77.88%   77.88%           
=======================================
  Files         163      163           
  Lines        8480     8480           
=======================================
  Hits         6605     6605           
  Misses       1875     1875           

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

Copy link
Contributor Author

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

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

Notes for reviewer:

Comment on lines -12 to -15
{% block page_desc %}{{ ftl('whatsnew-page-description') }}{% endblock %}

{#- This will appear as <meta property="og:description"> which can be used for social share -#}
{% block page_og_desc %}{{ ftl('whatsnew-page-description') }}{% endblock %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically the page_og_desc is only worth setting if it should differ from page_desc, otherwise self.page_desc() in base template takes care of that:

<meta property="og:description" content="{% filter striptags %}{% block page_og_desc %}{{ self.page_desc() }}{% endblock %}{% endfilter %}">

so setting page_desc properly is sufficient.

@@ -6,19 +6,10 @@

{% extends "firefox/whatsnew/base.html" %}

{% block page_title %}What’s new with Firefox{% endblock %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(This is the current -v2 string so that's now used in base and as a result these titles get also translated now…)

@@ -24,6 +24,8 @@
{% block body_id %}firefox-developer{% endblock %}
{% block body_class %}firefox-developer firefox-developer-firstrun{% endblock %}

{% block site_header %}{% endblock %}
Copy link
Contributor Author

@janbrasna janbrasna Jul 31, 2024

Choose a reason for hiding this comment

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

This now makes aurora in-product pages aligned with release/beta and nightly in a way that it doesn't show the main site header and navigation (wnp implicitly from the base diff here, firstrun explicitly on this line).

I've open #14949 for extra visibility.

@alexgibson alexgibson added Frontend HTML, CSS, JS... client side stuff Needs Review Awaiting code review P3 Third level priority - Nice to have labels Aug 19, 2024
@stephaniehobson stephaniehobson self-assigned this Sep 17, 2024
@janbrasna
Copy link
Contributor Author

Updated to include WNP 130–131 now, too.

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+ This looks great. Clean and through as always. Nice code cleanup.

@janbrasna In the opening comment you mention needed to add 131 nimbus URLs and further down you say 131 is done. Does that mean the 131 nimbus experiments are done too?

@janbrasna
Copy link
Contributor Author

@stephaniehobson That's a good catch! The template is covered, only at the time of the WIP comment there was a little bit of fiddling with the experiments vs. defaults/fallback etc. but this should be all sorted out now and I've included a bunch of experiment branches across regions in the testing links (which was the only thing missing — I never remember how the nimbus branches go into the query string;D…)

@stephaniehobson stephaniehobson merged commit 96dede2 into mozilla:main Oct 9, 2024
5 checks passed
@janbrasna janbrasna deleted the upd/wnp-base branch October 9, 2024 20:53
reemhamz pushed a commit that referenced this pull request Oct 10, 2024
* Move boilerplate to whatsnew/base

* Remove developer/firstrun masthead

* Add nightly/whatsnew description

* Prune old wnp assets

* Link fx short logo from img/logos
craigcook pushed a commit that referenced this pull request Oct 14, 2024
* Move boilerplate to whatsnew/base

* Remove developer/firstrun masthead

* Add nightly/whatsnew description

* Prune old wnp assets

* Link fx short logo from img/logos
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend HTML, CSS, JS... client side stuff Needs Review Awaiting code review P3 Third level priority - Nice to have
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FXDE in-product pages include navbar
3 participants