Skip to content

[Dynamic Dashboard] Fix date header layout to work better with large fonts #11552

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

Merged
merged 2 commits into from
May 20, 2024

Conversation

hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented May 20, 2024

Closes: #11551

Description

This PR just fixes the layout of the date header we use in the dynamic dashboard to work better with large fonts.

Testing instructions

  1. Enable large fonts on your phone (from the Accessibility or Display settings).
  2. Open the app.
  3. Make sure the Peformance card is enabled.
  4. Choose a custom range.
  5. Confirm the calendar button is still visible.

Images/gif

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@hichamboushaba hichamboushaba added type: bug A confirmed bug. feature: dashboard Related to home screen project labels May 20, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented May 20, 2024

1 Warning
⚠️ This PR is assigned to the milestone 18.7 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.
1 Message
📖 This PR contains changes to RELEASE-NOTES.txt.
Note that these changes won't affect the final version of the release notes as this version is in code freeze.
Please, get in touch with a release manager if you want to update the final release notes.

Generated by 🚫 Danger

@hichamboushaba hichamboushaba added this to the 18.7 ❄️ milestone May 20, 2024
Comment on lines +66 to +67
.weight(1f)
.wrapContentSize(align = Alignment.CenterStart)
Copy link
Member Author

Choose a reason for hiding this comment

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

weight will make sure this layout will have a lower priority than the IconButton below, and the wrapContentSize modifier will make sure the view's content won't fill the space when it's larger than the content.

@@ -72,7 +75,8 @@ fun DashboardStatsHeader(
MaterialTheme.colors.primary
} else {
MaterialTheme.colors.onSurface.copy(alpha = ContentAlpha.medium)
}
},
modifier = Modifier.weight(1f, fill = false)
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to make sure the Pencil icon is always visible.

@hichamboushaba hichamboushaba marked this pull request as ready for review May 20, 2024 17:34
@hichamboushaba hichamboushaba changed the title Improve the header's layout to work better with large fonts [Dynamic Dashboard] Fix date header layout to work better with large fonts May 20, 2024
@wpmobilebot
Copy link
Collaborator

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
FlavorJalapeno
Build TypeDebug
Commite9a92c5
Direct Downloadwoocommerce-prototype-build-pr11552-e9a92c5.apk

@codecov-commenter
Copy link

Codecov Report

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

Project coverage is 40.48%. Comparing base (c11910d) to head (e9a92c5).
Report is 1 commits behind head on release/18.7.

Files Patch % Lines
...android/ui/dashboard/stats/DashboardStatsHeader.kt 0.00% 4 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##             release/18.7   #11552      +/-   ##
==================================================
- Coverage           40.48%   40.48%   -0.01%     
  Complexity           5197     5197              
==================================================
  Files                1083     1083              
  Lines               62937    62939       +2     
  Branches             8629     8629              
==================================================
  Hits                25481    25481              
- Misses              35162    35164       +2     
  Partials             2294     2294              

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

@0nko 0nko self-assigned this May 20, 2024
Copy link
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

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

Works as expected, nice job! :shipit:

@0nko 0nko merged commit 9c57552 into release/18.7 May 20, 2024
16 checks passed
@0nko 0nko deleted the issue/11551-fix-stats-header branch May 20, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: dashboard Related to home screen project type: bug A confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants