[PM-11000] AnonLayout Icon/Logo theme refactor #10549

rr-bw · 2024-08-16T19:29:25Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-11000

📔 Objective

Adds a new tailwind class, marketing-logo, to handle automatically styling marketing related logos based on the theme. This allows us to remove the ThemeStateService, which was a bit cumbersome and required a workaround to check for the specific theme if the user had set their theme to system.

In this PR, the marketing-logo class is being used for both the Bitwarden Logo and the Bitwarden Shield Icon.

Note:

Removing the ThemeStateService means that Storybook will now show the correct color logo for both light and dark panels.

🎨 Storybook

https://62a88a6de5b807fa98886113-oxxsbpsfli.chromatic.com/?path=/story/auth-anon-layout--with-primary-content

📷 Screencast

web-themes.mov

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions · 2024-08-16T19:38:01Z

Checkmarx One – Scan Summary & Details – 09a14400-350a-4cea-a2c1-4078fe5d387e

No New Or Fixed Issues Found

codecov · 2024-08-16T19:40:37Z

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 33.13%. Comparing base (e7d4f85) to head (26386a1).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...h/src/angular/anon-layout/anon-layout.component.ts	0.00%	4 Missing ⚠️
...wrapper/extension-anon-layout-wrapper.component.ts	0.00%	3 Missing ⚠️
...on-layout-wrapper/extension-bitwarden-logo.icon.ts	0.00%	1 Missing ⚠️
...uth/src/angular/anon-layout/anon-layout.stories.ts	0.00%	1 Missing ⚠️
libs/auth/src/angular/icons/bitwarden-logo.icon.ts	0.00%	1 Missing ⚠️
...bs/auth/src/angular/icons/bitwarden-shield.icon.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10549   +/-   ##
=======================================
  Coverage   33.12%   33.13%           
=======================================
  Files        2687     2687           
  Lines       82975    82947   -28     
  Branches    15765    15756    -9     
=======================================
- Hits        27489    27482    -7     
+ Misses      53320    53299   -21     
  Partials     2166     2166

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

danielleflinn · 2024-08-16T21:01:41Z

Looks great! Thanks @rr-bw and @vleague2 for the collaboration on this

vleague2 · 2024-08-19T15:23:16Z

apps/desktop/desktop_native/Cargo.lock

+version = "1.7.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8318a53db07bb3f8dca91a600466bdb3f2eaadeedfdbcf02e1accbad9271ba50"
+


❓ Is this change supposed to be in here?

Yes this is fine, but not part of my changes. This happened on another PR and Dani gave some context here: #10492 (review)

I went ahead and merged the latest main into this PR which already has the cargo lock changes applied, so they are out of the diff. The one whitespace line in index.d.ts still remains as that has not been updated in main yet.

@quexten Thanks!

quexten · 2024-08-19T16:24:22Z

Nice work! Seems the linting automation failed because the github action had networking issues. Might want to just re-run the action.

vleague2 · 2024-08-19T18:17:55Z

libs/auth/src/angular/icons/bitwarden-shield.icon.ts

-    <path d="M82.2944 69.1899V37.2898H60V93.9624C63.948 91.869 67.4812 89.5927 70.5998 87.1338C78.3962 81.0196 82.2944 75.0383 82.2944 69.1899ZM91.8491 30.9097V69.1899C91.8491 72.0477 91.2934 74.8805 90.182 77.6883C89.0706 80.4962 87.6938 82.9884 86.0516 85.1649C84.4094 87.3415 82.452 89.4598 80.1794 91.5201C77.9068 93.5803 75.8084 95.2916 73.8842 96.654C71.96 98.0164 69.9528 99.304 67.8627 100.517C65.7726 101.73 64.288 102.552 63.4088 102.984C62.5297 103.416 61.8247 103.748 61.2939 103.981C60.8958 104.18 60.4645 104.28 60 104.28C59.5355 104.28 59.1042 104.18 58.7061 103.981C58.1753 103.748 57.4703 103.416 56.5911 102.984C55.712 102.552 54.2273 101.73 52.1372 100.517C50.0471 99.304 48.04 98.0164 46.1158 96.654C44.1916 95.2916 42.0932 93.5803 39.8206 91.5201C37.548 89.4598 35.5906 87.3415 33.9484 85.1649C32.3062 82.9884 30.9294 80.4962 29.818 77.6883C28.7066 74.8805 28.1509 72.0477 28.1509 69.1899V30.9097C28.1509 30.0458 28.4661 29.2981 29.0964 28.6668C29.7267 28.0354 30.4732 27.7197 31.3358 27.7197H88.6642C89.5268 27.7197 90.2732 28.0354 90.9036 28.6668C91.5339 29.2981 91.8491 30.0458 91.8491 30.9097Z" fill="#175DDC"/>
+export const BitwardenShield = svgIcon`
+  <svg viewBox="0 0 120 132" xmlns="http://www.w3.org/2000/svg">
+    <path class="tw-fill-marketing-logo" d="M82.2944 69.1899V37.2898H60V93.9624C63.948 91.869 67.4812 89.5927 70.5998 87.1338C78.3962 81.0196 82.2944 75.0383 82.2944 69.1899ZM91.8491 30.9097V69.1899C91.8491 72.0477 91.2934 74.8805 90.182 77.6883C89.0706 80.4962 87.6938 82.9884 86.0516 85.1649C84.4094 87.3415 82.452 89.4598 80.1794 91.5201C77.9068 93.5803 75.8084 95.2916 73.8842 96.654C71.96 98.0164 69.9528 99.304 67.8627 100.517C65.7726 101.73 64.288 102.552 63.4088 102.984C62.5297 103.416 61.8247 103.748 61.2939 103.981C60.8958 104.18 60.4645 104.28 60 104.28C59.5355 104.28 59.1042 104.18 58.7061 103.981C58.1753 103.748 57.4703 103.416 56.5911 102.984C55.712 102.552 54.2273 101.73 52.1372 100.517C50.0471 99.304 48.04 98.0164 46.1158 96.654C44.1916 95.2916 42.0932 93.5803 39.8206 91.5201C37.548 89.4598 35.5906 87.3415 33.9484 85.1649C32.3062 82.9884 30.9294 80.4962 29.818 77.6883C28.7066 74.8805 28.1509 72.0477 28.1509 69.1899V30.9097C28.1509 30.0458 28.4661 29.2981 29.0964 28.6668C29.7267 28.0354 30.4732 27.7197 31.3358 27.7197H88.6642C89.5268 27.7197 90.2732 28.0354 90.9036 28.6668C91.5339 29.2981 91.8491 30.0458 91.8491 30.9097Z" fill="#175DDC"/>


This still has a hardcoded hex for fill, should that be gone now that we are using the class?

Thanks for catching that! Fixed here: 5828e32

libs/components/src/tw-theme.css

rr-bw added 2 commits August 16, 2024 11:52

update base anon-layout logo/icon

db19517

update ExtensionAnonLayout logo/icon based on theme

38a0fa9

rr-bw marked this pull request as ready for review August 16, 2024 19:35

rr-bw requested review from a team as code owners August 16, 2024 19:35

rr-bw requested review from justindbaur, vleague2 and quexten August 16, 2024 19:35

rr-bw mentioned this pull request Aug 16, 2024

[PM-9314] AnonLayout Logo and Storybook Doc updates #9849

Merged

rr-bw changed the title ~~AnonLayout Icon/Logo theme refactor~~ [PM-11000] AnonLayout Icon/Logo theme refactor Aug 16, 2024

vleague2 reviewed Aug 19, 2024

View reviewed changes

Merge branch 'main' into auth/anon-layout-icon-theme-sync

b2279e0

quexten previously approved these changes Aug 19, 2024

View reviewed changes

vleague2 reviewed Aug 19, 2024

View reviewed changes

remove hard-coded fill

5828e32

rr-bw dismissed quexten’s stale review via 5828e32 August 19, 2024 18:33

rr-bw requested a review from vleague2 August 19, 2024 18:35

vleague2 reviewed Aug 21, 2024

View reviewed changes

libs/components/src/tw-theme.css Outdated Show resolved Hide resolved

rr-bw added 2 commits August 21, 2024 09:28

Merge branch 'main' into auth/anon-layout-icon-theme-sync

e165277

remove solarizedDark class

9233901

rr-bw requested a review from vleague2 August 21, 2024 16:30

vleague2 approved these changes Aug 21, 2024

View reviewed changes

rr-bw requested a review from quexten August 21, 2024 20:42

Merge branch 'main' into auth/anon-layout-icon-theme-sync

8f014ab

quexten approved these changes Aug 30, 2024

View reviewed changes

Merge branch 'main' into auth/anon-layout-icon-theme-sync

26386a1

rr-bw removed request for a team and justindbaur August 31, 2024 00:06

stefan0xC mentioned this pull request Aug 31, 2024

backport master password verification support dani-garcia/bw_web_builds#179

Merged

rr-bw merged commit c5a267b into main Aug 31, 2024
66 checks passed

rr-bw deleted the auth/anon-layout-icon-theme-sync branch August 31, 2024 19:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-11000] AnonLayout Icon/Logo theme refactor #10549

[PM-11000] AnonLayout Icon/Logo theme refactor #10549

rr-bw commented Aug 16, 2024 •

edited

Loading

github-actions bot commented Aug 16, 2024 •

edited

Loading

codecov bot commented Aug 16, 2024 •

edited

Loading

danielleflinn commented Aug 16, 2024

vleague2 Aug 19, 2024

rr-bw Aug 19, 2024

quexten Aug 19, 2024 •

edited

Loading

rr-bw Aug 19, 2024

quexten commented Aug 19, 2024

vleague2 Aug 19, 2024

rr-bw Aug 19, 2024

[PM-11000] AnonLayout Icon/Logo theme refactor #10549

[PM-11000] AnonLayout Icon/Logo theme refactor #10549

Conversation

rr-bw commented Aug 16, 2024 • edited Loading

🎟️ Tracking

📔 Objective

🎨 Storybook

📷 Screencast

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions bot commented Aug 16, 2024 • edited Loading

No New Or Fixed Issues Found

codecov bot commented Aug 16, 2024 • edited Loading

Codecov Report

danielleflinn commented Aug 16, 2024

vleague2 Aug 19, 2024

Choose a reason for hiding this comment

rr-bw Aug 19, 2024

Choose a reason for hiding this comment

quexten Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

rr-bw Aug 19, 2024

Choose a reason for hiding this comment

quexten commented Aug 19, 2024

vleague2 Aug 19, 2024

Choose a reason for hiding this comment

rr-bw Aug 19, 2024

Choose a reason for hiding this comment

rr-bw commented Aug 16, 2024 •

edited

Loading

github-actions bot commented Aug 16, 2024 •

edited

Loading

codecov bot commented Aug 16, 2024 •

edited

Loading

quexten Aug 19, 2024 •

edited

Loading