-
-
Notifications
You must be signed in to change notification settings - Fork 45
Fixed button spacing issue #3190
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
📝 WalkthroughWalkthroughThis change updates two XML layout files used in the application's UI. In Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant CardView
participant LinearLayout
participant TextView
participant MaterialButton
User->>UI: Opens delivery details or job intro fragment
UI->>CardView: Inflate CardView layout
CardView->>LinearLayout: Initialize vertical LinearLayout
LinearLayout->>TextView: Display action title/label
LinearLayout->>TextView: Display action details/summary
LinearLayout->>MaterialButton: Display action button (aligned to end)
User->>MaterialButton: Interact with button
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/res/layout/fragment_connect_job_intro.xml (1)
99-99: Align button and margins consistently
android:layout_gravity="end"correctly right-aligns the button, but pairing it withandroid:layout_marginStartis confusing. If you intend spacing from the end edge, switch toandroid:layout_marginEnd.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/res/layout/fragment_connect_delivery_details.xml(2 hunks)app/res/layout/fragment_connect_job_intro.xml(1 hunks)
🔇 Additional comments (3)
app/res/layout/fragment_connect_job_intro.xml (3)
76-76: Orientation update is correct
Changing the innerLinearLayoutto vertical prevents overlap and supports longer translations by stacking text and button.
79-85: Static start label looks good
The newTextViewforconnect_job_intro_start_labelis well scoped withwrap_contentand proper styling.
87-93: Verify dynamic text assignment
ThisTextView(connect_job_intro_learning_summary) has noandroid:textin XML. Ensure its content is set in code or add atools:textplaceholder, otherwise it will render blank.
| <LinearLayout | ||
| android:id="@+id/linearLayout" | ||
| android:layout_width="0dp" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:orientation="vertical" | ||
| app:layout_constraintTop_toTopOf="parent" | ||
| app:layout_constraintBottom_toBottomOf="parent" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| android:layout_marginEnd="2dp" | ||
| app:layout_constraintEnd_toStartOf="@+id/connect_delivery_button" | ||
| app:layout_constraintTop_toTopOf="parent"> | ||
| android:background="@color/connect_darker_blue_color"> |
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.
🛠️ Refactor suggestion
Flatten redundant wrappers
You’ve introduced a LinearLayout inside a ConstraintLayout and duplicated the same background on both. Consider making the LinearLayout the direct child of the CardView (removing the inner ConstraintLayout) and drop the extra background on one of them to simplify the view hierarchy.
🤖 Prompt for AI Agents
In app/res/layout/fragment_connect_delivery_details.xml around lines 163 to 172,
there is a redundant nested layout where a LinearLayout is inside a
ConstraintLayout with duplicated background attributes. To fix this, remove the
inner ConstraintLayout and make the LinearLayout the direct child of the
CardView, then remove the duplicate background attribute from one of the layouts
to simplify the view hierarchy and avoid unnecessary nesting.
| <com.google.android.material.button.MaterialButton | ||
| android:id="@+id/connect_delivery_button" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:text="@string/connect_job_info_download" | ||
| app:layout_constraintTop_toBottomOf="@id/connect_delivery_action_details" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| android:layout_gravity="end" | ||
| android:backgroundTint= "@color/white" | ||
| android:drawableEnd= "@drawable/ic_connect_arrow_forward_20px" | ||
| android:textColor= "@color/cc_brand_color" | ||
| app:iconTint="@color/cc_brand_color" | ||
| /> |
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.
🛠️ Refactor suggestion
Clean up misplaced constraint attributes
The MaterialButton is now inside a vertical LinearLayout, so the app:layout_constraint* attributes have no effect and should be removed. Also adjust margins—use android:layout_marginEnd for end-side spacing—and drop any unused android:gravity on the button.
🤖 Prompt for AI Agents
In app/res/layout/fragment_connect_delivery_details.xml between lines 195 and
207, remove the constraint layout attributes app:layout_constraintTop_toBottomOf
and app:layout_constraintEnd_toEndOf from the MaterialButton since it is inside
a LinearLayout where constraints do not apply. Replace any spacing adjustments
with android:layout_marginEnd for proper end margin. Also, remove the
android:layout_gravity attribute from the button as it is unnecessary inside the
LinearLayout.
…in some languages
https://dimagi.atlassian.net/browse/QA-7848
https://dimagi.atlassian.net/browse/QA-7847
On the Job Intro and Job Details pages, the button to proceed was to the right of some text.
On the Learning/Delivery Progress pages, the sync button was similar
On the Phone Verification page, similar again with the Change link
When translated to French it all gets bigger, and causes strange overflows.
I rearranged these components vertically to allow room for them to expand horizontally, such that rollover will be isolated to each component rather than squishing other components.
Example of the new look:
