-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Adjust “What's new” modal UI and items for tablet #7605
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: feature/cradoiu/whats-new-top-section-scrollable
Are you sure you want to change the base?
Adjust “What's new” modal UI and items for tablet #7605
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
...ssaging/remote-messaging-impl/src/main/res/layout/view_remote_message_featured_list_item.xml
Outdated
Show resolved
Hide resolved
remote-messaging/remote-messaging-impl/src/main/res/layout/view_remote_message_card_item.xml
Outdated
Show resolved
Hide resolved
...ssaging/remote-messaging-impl/src/main/res/layout/view_remote_message_section_title_item.xml
Show resolved
Hide resolved
remote-messaging/remote-messaging-impl/src/main/res/layout/view_remote_message_card_item.xml
Show resolved
Hide resolved
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.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
| <androidx.constraintlayout.widget.ConstraintLayout | ||
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" | ||
| android:paddingBottom="@dimen/featuredListItemBottomMargin"> |
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.
Inner card padding uses wrong dimension resource
Medium Severity
The inner ConstraintLayout's paddingBottom was changed from @dimen/keyline_2 (8dp) to @dimen/featuredListItemBottomMargin. This dimension is meant for spacing between list items (used correctly on the outer wrapper at line 25), not for internal card content padding. On tablets this results in 40dp of internal bottom padding instead of 8dp, creating excessive whitespace inside the featured card. The inner padding likely needs to remain at @dimen/keyline_2.
Please tell me if this was useful or not with a 👍 or 👎.
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.
This is according to the Figma design.
| android:layout_marginStart="@dimen/actionButtonHorizontalMargin" | ||
| android:layout_marginEnd="@dimen/actionButtonHorizontalMargin" | ||
| android:layout_marginTop="16dp" | ||
| android:layout_marginEnd="@dimen/keyline_5" |
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.
Action button missing max width constraint for tablet consistency
Low Severity
The actionButton is missing app:layout_constraintWidth_max="@dimen/messageContentMaxWidth" that was added to all other message components in this PR. On large tablets/foldables (e.g., 1200dp width), the list items above will be constrained to 600dp and centered, while the button stretches to approximately 1080dp, creating visual inconsistency. This appears to be an oversight given the PR's stated goal of adding max width constraints to prevent excessive stretching.
Please tell me if this was useful or not with a 👍 or 👎.
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.
This is according to the Figma design.

Task/Issue URL: https://app.asana.com/1/137249556945/project/1211724162604201/task/1212898863116333?focus=true
Description
This PR improves the “What’s new” modal UI for tablet devices by:
Steps to test this PR
Check the steps inside the following task to the the changes: https://app.asana.com/1/137249556945/project/1211724162604201/task/1213012607800059?focus=true
UI changes
Before changes
|
After changes
Note
Improves layout scalability of the “What’s new” modal, especially on tablets.
messageContentMaxWidth(600dp) and applies it to header, section title, featured card, and list itemsvaluesandvalues-sw600dp) to adjust margins/padding (e.g.,actionButtonHorizontalMargin, featured item spacing)listItemContainerin card items; restructured featured card and header wrappers)listItemContainerinstead of root inCardsListAdapterWritten by Cursor Bugbot for commit c3f50b4. This will update automatically on new commits. Configure here.