-
Notifications
You must be signed in to change notification settings - Fork 927
PM-27497: Update Snackbar font when there is no header #6086
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
Conversation
|
Claude finished @david-livefront's task —— View job Code Review CompleteTasks
Summary of ChangesThis PR updates the Changed File: Critical IssuesNone identified. Suggested Improvements1. Missing Test Coverage for the New Conditional LogicSeverity: Medium The PR introduces conditional typography logic but doesn't include unit tests to verify the behavior. The component lacks dedicated test files, making it difficult to ensure the styling change works correctly in both scenarios. RecommendationConsider adding screenshot tests or snapshot tests to verify:
This would prevent regressions and document the intended behavior. While UI module tests may be structured differently, having some form of automated verification would be valuable for this visual change. 2. Preview Function Could Demonstrate Both StatesSeverity: Low The existing preview function Example@Preview
@Composable
private fun BitwardenCustomSnackbar_withoutHeader_preview() {
BitwardenTheme {
Surface {
BitwardenSnackbar(
BitwardenSnackbarData(
message = "Message without header".asText(),
actionLabel = "Action".asText(),
withDismissAction = true,
),
)
}
}
}This would make it easier for reviewers and designers to verify the visual changes. 3. Consider Documenting the Typography DecisionSeverity: Low The inline comment "Upgrade the font when it is stand alone" (line 96) is helpful, but it would be even better to document this design decision in the KDoc for the function or in a comment explaining the UX rationale (e.g., "standalone messages need more visual prominence"). Good Practices Observed
Action Items for Author
Architecture & Style Compliance✅ Architecture: Change is localized to the UI component and follows Compose best practices Overall AssessmentThis is a straightforward, well-implemented UI improvement that enhances the visual hierarchy of standalone snackbar messages. The change is minimal, focused, and follows existing patterns. The main suggestions are around adding test coverage and preview functions to prevent regressions and improve documentation. Recommendation: Approve with optional improvements suggested above. |
|
Thanks @SaintPatrck |
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6086 +/- ##
========================================
Coverage 84.81% 84.81%
========================================
Files 721 738 +17
Lines 52830 53193 +363
Branches 7671 7673 +2
========================================
+ Hits 44809 45117 +308
- Misses 5331 5386 +55
Partials 2690 2690 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|

🎟️ Tracking
PM-27497
📔 Objective
This PR updates the Snackbar font for the message when there is no header present.
📸 Screenshots
⏰ Reminders before review
🦮 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