-
Notifications
You must be signed in to change notification settings - Fork 946
Update bedrock to Protocol V22 #16586
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
base: main
Are you sure you want to change the base?
Conversation
d6cfc20 to
fc3a9f5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16586 +/- ##
=======================================
Coverage 79.70% 79.70%
=======================================
Files 159 159
Lines 8534 8534
=======================================
Hits 6802 6802
Misses 1732 1732 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code review is r+wc, going to do browser review tomorrow morning
the non-blocking requested change is to use @use instead of @import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the play/pause icons, the skip pixel of yesteryear was obnoxious;]
There's also two icons on products.landing that are outdated now:
bedrock/bedrock/products/templates/products/landing.html
Lines 38 to 39 in a59a279
| {% set icon_external = '<span class="mzp-c-button-icon-end"><svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><g fill="none" fill-rule="evenodd" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2.5"><path d="M13 4h7v7M12 12l8-8M8 4H7a3 3 0 0 0-3 3v10a3 3 0 0 0 3 3h10a3 3 0 0 0 3-3v-1"/></g></svg></span>' %} | |
| {% set icon_download ='<span class="mzp-c-button-icon-end"><svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" viewBox="0 0 16 16" width="16" height="16" fill="currentColor"><path d="M8 13c.2 0 .4-.1.5-.2l4.4-4.4-1.1-1.1-3.1 3.1V1H7.2v9.4L4.1 7.3l-1 1.1 4.4 4.4c.1.1.3.2.5.2z" /><path d="M13.5 12v2c0 .3-.2.5-.5.5H3c-.3 0-.5-.2-.5-.5v-2H1v2c0 1.1.9 2 2 2h10c1.1 0 2-.9 2-2v-2h-1.5z" /></svg></span>' %} |
Some random notes:
| <title>Download</title> | ||
| <path fill="currentColor" d="M19.5 17.31v2.13H4.72v-2.13h-2.6v4.73H22.1v-4.73h-2.6z"/> | ||
| <path fill="currentColor" d="M21.44 7.09h-8.03V2.06h-2.6v5.03H2.78l9.33 11.09 9.33-11.09Zm-5.59 2.59-3.74 4.46-3.74-4.46h7.48Z"/> | ||
| </svg></span></a> | ||
| <a download class="mzp-c-button" href="https://assets.mozilla.net/pdf/transparency-report/2024-dsa.pdf">2024 DSA Transparency Report <span class="mzp-c-button-icon-end"><svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"><clipPath id="a"><path d="M0 0h16v16H0z"/></clipPath><g fill="currentColor" clip-path="url(#a)"><path d="M7.25 0v10.21L3.53 6.49 2.47 7.55 8 13.08l5.53-5.53-1.06-1.06-3.72 3.72V0zM16 14.5H0V16h16z"/></g></svg></a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems the title here inside the SVG got removed?
BTW, the download attribute does something?
| if ftl_file_is_active("mozorg/home-m24") and experience != "legacy": | ||
| if ftl_file_is_active("mozorg/home-m24") and experience not in ["quantum", "trailhead"]: | ||
| return [self.m24_template_name] | ||
| elif ftl_file_is_active("mozorg/home-new") and experience != "legacy": | ||
| elif ftl_file_is_active("mozorg/home-new") and experience != "quantum": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay this makes sense (I wish there was also a way to trigger the nav/footer from that time) — but wondering if that needs a comment, or a documentation somewhere?
(I remember learning about the xv=legacy at some point, seemingly within ftl file comments; but can't find that any longer. Wondering where was the learning curve for this.)
NB: the experience value for mozorg/home is something different than the firefox/download already documented in mkdocs.
| &.is-hidden { | ||
| display: none; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC this is extra, when history state is empty?
| &.is-hidden { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Hmm here too, for some reason, was that for long vs. short title?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK can't find this any longer — it would have to come from CMS somehow.
One-line summary
Update bedrock to Protocol V22
Significant changes and points to review
V22 changelog
Issue / Bugzilla link
mozilla/protocol#1074
#13462
Testing
Color token changes:
Breadcrumbs/subnav:
caret -> arrow icon updates (mostly modal navigation):
rich text & icon updates
Integration test run