PM-15147 - Design Audit - Master Password Guidance Screen #4383

phil-livefront · 2024-11-25T15:32:10Z

🎟️ Tracking

PM-15147

📔 Objective

Align Text: Center-align the title and subtitle text across relevant components.
Design Updates: Update the content block styling to match the Figma designs for V3.
Background Changes: Remove the blue background from the "What makes a password strong?" block.
Registration Screen Adjustments: Remove the blue content background from the CompleteRegistrationScreen where the MasterPasswordGuidanceScreen is launched. This follows iOS design changes. Retain the border color to minimize deviations.
Action Card Enhancement: Modify BitwardenActionCard to include an optional dismiss action, supporting scenarios like the MasterPasswordGuidanceScreen where no dismiss action is required.

📸 Screenshots

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation 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

…uidance-screen

github-actions · 2024-11-25T15:37:51Z

Checkmarx One – Scan Summary & Details – b5865413-87af-47fc-a148-2af268d79ec7

New Issues

Issue	Source File / Package	Checkmarx Insight
Privacy_Violation	/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt: 346	Attack Vector
Privacy_Violation	/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt: 346	Attack Vector
Privacy_Violation	/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModel.kt: 321	Attack Vector
Privacy_Violation	/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModel.kt: 321	Attack Vector
Privacy_Violation	/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt: 875	Attack Vector
Unpinned Actions Full Length Commit SHA	/crowdin-push.yml: 26	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Fixed Issues

Severity	Issue	Source File / Package
	Privacy_Violation	/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt: 341
	Privacy_Violation	/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt: 341
	Unpinned Actions Full Length Commit SHA	/crowdin-pull.yml: 26

codecov · 2024-11-25T17:44:38Z

Codecov Report

Attention: Patch coverage is 98.63014% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.01%. Comparing base (bca9f5e) to head (c628d2f).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...erpasswordguidance/MasterPasswordGuidanceScreen.kt	98.57%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4383      +/-   ##
==========================================
- Coverage   89.01%   89.01%   -0.01%     
==========================================
  Files         451      451              
  Lines       39123    39129       +6     
  Branches     5532     5533       +1     
==========================================
+ Hits        34827    34832       +5     
  Misses       2368     2368              
- Partials     1928     1929       +1

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

dseverns-livefront · 2024-11-27T19:38:17Z

app/src/main/java/com/x8bit/bitwarden/ui/platform/components/card/BitwardenActionCard.kt

-                contentDescription = stringResource(id = R.string.close),
-                onClick = onDismissClick,
-            )
+            onDismissClick?.let {


dseverns-livefront · 2024-11-27T19:38:28Z

app/src/main/java/com/x8bit/bitwarden/ui/platform/components/card/BitwardenActionCard.kt

+@Preview
+@Preview(uiMode = Configuration.UI_MODE_NIGHT_YES)
+@Composable
+private fun BitwardenActionCardWithSubtitleNoDismiss_preview() {


dseverns-livefront · 2024-11-27T19:42:10Z

...a/com/x8bit/bitwarden/ui/auth/feature/masterpasswordguidance/MasterPasswordGuidanceScreen.kt

+                .fillMaxWidth()
+                .padding(horizontal = 16.dp),
+        )
+        Spacer(modifier = Modifier.height(24.dp))


⛏️ maybe minor thing, curious what others thoughts are with this, maybe we can just remove the wrapping Columns and have it so at the call site its like

Column { ContentItem1() Spacer() ContentItem2() etc. }

yeah i like this idea better, ill make the change!

dseverns-livefront · 2024-11-27T19:42:53Z

...a/com/x8bit/bitwarden/ui/auth/feature/masterpasswordguidance/MasterPasswordGuidanceScreen.kt

+        cardTitle = stringResource(R.string.need_some_inspiration),
+        actionText = stringResource(R.string.check_out_the_passphrase_generator),
+        onActionClick = onActionClicked,
+        modifier = modifier


⛏️ this is Okay as is, but could also just be same line.

ah okay, yeah one liner looks more "in-line" zing

dseverns-livefront

Just some style nits, but otherwise looks GR8

david-livefront · 2024-12-02T17:04:56Z

...a/com/x8bit/bitwarden/ui/auth/feature/masterpasswordguidance/MasterPasswordGuidanceScreen.kt

@@ -81,126 +78,134 @@ fun MasterPasswordGuidanceScreen(
            )
        },
    ) {
-        Column(
+        MasterPasswordGuidanceContent(


Is the LongMethod suppression above still needed?

david-livefront · 2024-12-02T17:06:02Z

...a/com/x8bit/bitwarden/ui/auth/feature/masterpasswordguidance/MasterPasswordGuidanceScreen.kt

-            )
-            Spacer(modifier = Modifier.navigationBarsPadding())
-        }
+            viewModel = viewModel,


Don't pass the whole VM into the content, just pass in the state and any lambdas needed.

okay sounds good! Just curious, is there a downside to passing in the viewModel or just a style thing?

actually after implementing the change it became much clearer to me why this is a better approach. Thanks!

It can also help with previews and individual composable tests (even if we rarely do those)

david-livefront · 2024-12-02T17:08:03Z

app/src/main/java/com/x8bit/bitwarden/ui/platform/components/card/BitwardenActionCard.kt

@@ -44,7 +44,7 @@ fun BitwardenActionCard(
    cardTitle: String,
    actionText: String,
    onActionClick: () -> Unit,
-    onDismissClick: () -> Unit,
+    onDismissClick: (() -> Unit)? = null,


Can we move this below the modifier, the pattern is that the modifier is always the first param with a default value

…uidance-screen

dseverns-livefront

david-livefront · 2024-12-02T19:49:47Z

...a/com/x8bit/bitwarden/ui/auth/feature/masterpasswordguidance/MasterPasswordGuidanceScreen.kt

-            )
-            Spacer(modifier = Modifier.navigationBarsPadding())
-        }
+            onTryPasswordGeneratorAction = {


Be sure to remeber the viewModel

david-livefront · 2024-12-02T19:50:10Z

...a/com/x8bit/bitwarden/ui/auth/feature/masterpasswordguidance/MasterPasswordGuidanceScreen.kt

    modifier: Modifier = Modifier,
+    onTryPasswordGeneratorAction: () -> Unit,


This should be before the modifier.

phil-livefront added 2 commits November 25, 2024 10:25

PM-15147 - Design Audit - Master Password Guidance Screen

11c8711

Merge branch 'main' into phil/PM-15147-design-audit-master-password-g…

8fb81e9

…uidance-screen

phil-livefront added 2 commits November 25, 2024 11:37

fix linter errors

63ce644

fix texts and abstract missed strings to resource

47c5e86

phil-livefront marked this pull request as ready for review November 25, 2024 17:41

phil-livefront requested review from brian-livefront, david-livefront, dseverns-livefront, ahaisting-livefront and a team as code owners November 25, 2024 17:41

phil-livefront enabled auto-merge November 25, 2024 19:21

dseverns-livefront reviewed Nov 27, 2024

View reviewed changes

david-livefront reviewed Dec 2, 2024

View reviewed changes

phil-livefront added 2 commits December 2, 2024 12:12

Merge branch 'main' into phil/PM-15147-design-audit-master-password-g…

79a9d56

…uidance-screen

PR comments

c628d2f

dseverns-livefront approved these changes Dec 2, 2024

View reviewed changes

phil-livefront added this pull request to the merge queue Dec 2, 2024

david-livefront reviewed Dec 2, 2024

View reviewed changes

Merged via the queue into main with commit b2391dd Dec 2, 2024
9 checks passed

phil-livefront deleted the phil/PM-15147-design-audit-master-password-guidance-screen branch December 2, 2024 20:04

phil-livefront mentioned this pull request Dec 3, 2024

PM-15147 - MasterPasswordGuidanceScreen PR Cleanup #4411

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PM-15147 - Design Audit - Master Password Guidance Screen #4383

PM-15147 - Design Audit - Master Password Guidance Screen #4383

phil-livefront commented Nov 25, 2024 •

edited by jira bot

Loading

github-actions bot commented Nov 25, 2024 •

edited

Loading

codecov bot commented Nov 25, 2024 •

edited

Loading

dseverns-livefront Nov 27, 2024

dseverns-livefront Nov 27, 2024

dseverns-livefront Nov 27, 2024

phil-livefront Dec 2, 2024

dseverns-livefront Nov 27, 2024

phil-livefront Dec 2, 2024

dseverns-livefront left a comment

david-livefront Dec 2, 2024

david-livefront Dec 2, 2024

phil-livefront Dec 2, 2024

phil-livefront Dec 2, 2024

david-livefront Dec 2, 2024

david-livefront Dec 2, 2024

dseverns-livefront left a comment

david-livefront Dec 2, 2024 •

edited

Loading

david-livefront Dec 2, 2024

		modifier: Modifier = Modifier,
		onTryPasswordGeneratorAction: () -> Unit,

PM-15147 - Design Audit - Master Password Guidance Screen #4383

PM-15147 - Design Audit - Master Password Guidance Screen #4383

Conversation

phil-livefront commented Nov 25, 2024 • edited by jira bot Loading

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions bot commented Nov 25, 2024 • edited Loading

New Issues

Fixed Issues

codecov bot commented Nov 25, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dseverns-livefront left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dseverns-livefront left a comment

Choose a reason for hiding this comment

david-livefront Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phil-livefront commented Nov 25, 2024 •

edited by jira bot

Loading

github-actions bot commented Nov 25, 2024 •

edited

Loading

codecov bot commented Nov 25, 2024 •

edited

Loading

david-livefront Dec 2, 2024 •

edited

Loading