Skip to content

Commit

Permalink
Fixed #1956: Optimise code in profile chooser add view (#4844)
Browse files Browse the repository at this point in the history
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixed #1956: Optimise code in profile chooser add view

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [ ] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

| Screenshots Before |  After
|:-------------------------:|:-------------------------:|
| Mobile Portrait |

|![mobile_portrait_before](https://user-images.githubusercontent.com/43546652/206875860-745fe9bb-d9e6-4951-961f-89115567c778.png)
|
![mobile_portrait_after](https://user-images.githubusercontent.com/43546652/206875863-14733b63-8c09-4626-b89d-ebb55dfe6ba7.png)
| Mobile Landscape |

|![mobile_land_before](https://user-images.githubusercontent.com/43546652/206875865-b14b6106-9d8e-4890-ac67-038c23b2b2e9.png)
|
![mobile_land_after](https://user-images.githubusercontent.com/43546652/206876484-0115c672-ddf4-40f5-980d-c0119e318a38.png)
| Tap Portrait |
|
![tap_portrait_before](https://user-images.githubusercontent.com/43546652/206875870-4051acbe-5c46-47c8-8d79-cfc4cd4f4a38.png)
|
![tap_portrait_after](https://user-images.githubusercontent.com/43546652/206875871-7f70d5e6-7a8a-4bfa-bc96-cc49ee7a4539.png)
| Tap Landscape |

|![tap_land2_before](https://user-images.githubusercontent.com/43546652/206876590-0386cc8e-a3d5-43f7-bf11-4ea0ad48fb48.png)
|
![tap_land2_after](https://user-images.githubusercontent.com/43546652/206875868-58d33fcb-6c67-4300-b887-dd8dd16ca586.png)
| Tap LTR Portrait |
|
![tap_lrt_portrait_before](https://user-images.githubusercontent.com/43546652/206875874-4b0878a1-3d78-4dfc-8c90-473d1cefe7f7.png)
|
![tap_ltr_portrait_after](https://user-images.githubusercontent.com/43546652/206875876-fb42684f-f8b2-41bc-83a7-ea6d58322cb6.png)
| Tap LTR Landscape |

|![tap_ltr_land_before](https://user-images.githubusercontent.com/43546652/206875877-40949643-ac03-4743-a0ae-5a266a67490a.png)
|
![tap_ltr_land_after](https://user-images.githubusercontent.com/43546652/206875879-bd9cc1f6-5f4c-48b8-a345-3781f82bab67.png)
| Mobile LTR Portrait |

|![mobile_ltr_before](https://user-images.githubusercontent.com/43546652/206875880-717ab192-ba03-4b88-821d-21e1500bf301.png)
|
![mobile_ltr_portrait_after](https://user-images.githubusercontent.com/43546652/206875881-a9fc3334-7960-4401-a681-7e4472fd2f17.png)
| Mobile LTR Landscape |

|![mobile_ltr_land_before](https://user-images.githubusercontent.com/43546652/206875882-c8033397-620d-411f-b3ec-353959320355.png)
|
![mobile_ltr_land_after](https://user-images.githubusercontent.com/43546652/206875883-36b9b7bd-ea85-41c9-b74f-e0cf85e6baf3.png)

## Accessibility Guide 

https://user-images.githubusercontent.com/43546652/206877043-0e4221bc-b991-4b4d-8ae7-785eac6d4644.mov


## Unit Test Screenshot
<img width="852" alt="Screenshot 2022-12-09 at 7 52 10 AM"
src="https://user-images.githubusercontent.com/43546652/206876749-bda902b6-21b1-4102-8ace-f0fe41bf9877.png">
  • Loading branch information
Uticodes authored Jan 30, 2023
1 parent 49a6c32 commit 2dfad61
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 98 deletions.
26 changes: 12 additions & 14 deletions app/src/main/res/layout-land/profile_chooser_add_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
type="androidx.databinding.ObservableField&lt;Boolean&gt;" />
</data>

<LinearLayout
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="horizontal">
Expand All @@ -26,40 +26,38 @@
android:gravity="center_horizontal"
android:orientation="vertical"
app:layoutMarginBottom="@{hasProfileEverBeenAddedValue ? @dimen/profile_view_already_added_margin : @dimen/profile_chooser_add_view_margin_bottom_profile_not_added}"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_chooser_add_view_margin_top_profile_already_added : @dimen/profile_chooser_add_view_margin_top_profile_not_added}">
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_chooser_add_view_margin_top_profile_already_added : @dimen/profile_chooser_add_view_margin_top_profile_not_added}"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent">

<com.google.android.material.imageview.ShapeableImageView
android:id="@+id/profile_add_button"
android:layout_width="72dp"
android:layout_height="72dp"
android:layout_marginBottom="8dp"
app:srcCompat="@{@drawable/ic_add_profile}"
app:shapeAppearanceOverlay="@style/ShapeAppearanceOverlay.RoundedShape"
app:srcCompat="@{@drawable/ic_add_profile}"
app:strokeColor="@color/component_color_profile_chooser_activity_avatar_border_color"
app:strokeWidth="1dp" />

<TextView
android:id="@+id/add_profile_text"
style="@style/TextViewCenter"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
style="@style/Caption"
android:layout_gravity="center"
android:fontFamily="sans-serif-medium"
android:text="@{hasProfileEverBeenAddedValue ? @string/profile_chooser_add : @string/set_up_multiple_profiles}"
android:textColor="@color/component_color_profile_chooser_activity_white_text_color"
android:textSize="14sp" />
android:textColor="@color/component_color_shared_secondary_4_text_color" />

<TextView
android:id="@+id/add_profile_description_text"
style="@style/TextViewCenter"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
style="@style/TextViewStart"
android:layout_gravity="center"
android:fontFamily="sans-serif"
android:text="@string/set_up_multiple_profiles_description"
android:textColor="@color/component_color_profile_chooser_activity_white_text_color"
android:textAlignment="center"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="12sp"
android:visibility="@{hasProfileEverBeenAddedValue ? View.GONE : View.VISIBLE}" />
</LinearLayout>
</LinearLayout>
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
12 changes: 7 additions & 5 deletions app/src/main/res/layout-land/profile_chooser_profile_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
name="viewModel"
type="org.oppia.android.app.model.ProfileChooserUiModel" />
</data>

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">
Expand All @@ -39,17 +40,18 @@
android:id="@+id/profile_avatar_img"
android:layout_width="72dp"
android:layout_height="72dp"
android:layout_marginBottom="8dp"
app:shapeAppearanceOverlay="@style/ShapeAppearanceOverlay.RoundedShape"
app:strokeColor="@color/component_color_profile_chooser_activity_avatar_border_color"
app:strokeWidth="1dp"
profile:src="@{viewModel.profile.avatar}" />
android:layout_marginBottom="8dp"
profile:src="@{viewModel.profile.avatar}"
app:strokeColor="@color/component_color_shared_divider_color"
app:shapeAppearanceOverlay="@style/ShapeAppearanceOverlay.RoundedShape" />

<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="center_horizontal"
android:orientation="vertical">

<TextView
android:id="@+id/profile_name_text"
style="@style/Caption"
Expand Down Expand Up @@ -84,8 +86,8 @@
android:id="@+id/add_profile_divider_view"
android:layout_width="1dp"
android:layout_height="match_parent"
android:layout_marginTop="@dimen/profile_chooser_profile_view_view_margin_top"
android:layout_gravity="bottom"
android:layout_marginTop="@dimen/profile_chooser_profile_view_view_margin_top"
android:background="@color/component_color_shared_divider_color"
android:visibility="@{hasProfileEverBeenAddedValue ? View.GONE : View.VISIBLE}"
app:layout_constraintBottom_toBottomOf="@+id/profile_chooser_item"
Expand Down
45 changes: 21 additions & 24 deletions app/src/main/res/layout-sw600dp-land/profile_chooser_add_view.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:tools="http://schemas.android.com/tools"
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto">
<layout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools">

<data>

Expand All @@ -16,67 +16,64 @@
type="androidx.databinding.ObservableField&lt;Boolean&gt;" />
</data>

<LinearLayout
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="center_horizontal"
android:layout_margin="@dimen/profile_chooser_profile_view_margin"
android:padding="@dimen/profile_chooser_profile_view_parent_padding"
android:paddingStart="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_parent_margin_start_profile_not_added}"
android:paddingEnd="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_parent_margin_end_profile_not_added}"
android:orientation="vertical">
app:layoutMarginBottom="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/space_0dp}">

<LinearLayout
android:id="@+id/add_profile_item"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_top_profile_not_added}"
android:gravity="@{hasProfileEverBeenAddedValue ? Gravity.CENTER_HORIZONTAL : Gravity.CENTER}"
android:orientation="@{hasProfileEverBeenAddedValue ? LinearLayout.VERTICAL : LinearLayout.HORIZONTAL}"
android:paddingEnd="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_end_profile_not_added}"
android:paddingStart="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_start_profile_not_added}"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_top_profile_not_added}"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
tools:ignore="RtlSymmetry">

<com.google.android.material.imageview.ShapeableImageView
android:id="@+id/profile_add_button"
android:layout_width="108dp"
android:layout_height="108dp"
app:srcCompat="@{@drawable/ic_add_profile}"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_chooser_add_view_circular_image_margin_top_profile_already_added : @dimen/space_0dp}"
app:strokeColor="@color/component_color_profile_chooser_activity_avatar_border_color"
app:strokeWidth="1dp"
app:shapeAppearanceOverlay="@style/ShapeAppearanceOverlay.RoundedShape" />
app:strokeColor="@color/component_color_shared_divider_color"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_chooser_add_view_circular_image_margin_top_profile_already_added : @dimen/space_0dp}"
app:srcCompat="@{@drawable/ic_add_profile}"
app:shapeAppearanceOverlay="@style/ShapeAppearanceOverlay.RoundedShape"/>

<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="@{hasProfileEverBeenAddedValue ? Gravity.CENTER_HORIZONTAL : Gravity.CENTER_VERTICAL}"
android:orientation="vertical"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_view_already_added_description_parent_margin_top : @dimen/space_0dp}"
android:paddingStart="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_description_margin_start_profile_not_added}"
android:paddingEnd="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_description_margin_end_profile_not_added}"
android:paddingStart="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_description_margin_start_profile_not_added}">
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_view_already_added_description_parent_margin_top : @dimen/space_0dp}">

<TextView
android:id="@+id/add_profile_text"
style="@style/TextViewStart"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:fontFamily="sans-serif-medium"
style="@style/Caption"
android:text="@{hasProfileEverBeenAddedValue ? @string/profile_chooser_add : @string/set_up_multiple_profiles}"
android:textColor="@color/component_color_profile_chooser_activity_white_text_color"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="20sp" />

<TextView
android:id="@+id/add_profile_description_text"
style="@style/TextViewCenter"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
style="@style/TextViewStart"
android:layout_gravity="center"
android:layout_marginTop="4dp"
android:fontFamily="sans-serif"
android:text="@string/set_up_multiple_profiles_description"
android:textColor="@color/component_color_profile_chooser_activity_white_text_color"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="16sp"
android:visibility="@{hasProfileEverBeenAddedValue ? View.GONE : View.VISIBLE}" />
</LinearLayout>
</LinearLayout>
</LinearLayout>
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
45 changes: 20 additions & 25 deletions app/src/main/res/layout-sw600dp-port/profile_chooser_add_view.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:tools="http://schemas.android.com/tools"
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto">
<layout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools">

<data>

Expand All @@ -16,68 +16,63 @@
type="androidx.databinding.ObservableField&lt;Boolean&gt;" />
</data>

<LinearLayout
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_margin="@dimen/profile_chooser_profile_view_margin"
android:gravity="center_horizontal"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_view_already_added_description_parent_margin_top : @dimen/space_0dp}"
android:paddingEnd="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_description_margin_end_profile_not_added}"
android:paddingStart="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_description_margin_start_profile_not_added}"
android:orientation="vertical">
android:padding="@dimen/profile_chooser_profile_view_parent_padding">

<LinearLayout
android:id="@+id/add_profile_item"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_top_profile_not_added}"
android:gravity="@{hasProfileEverBeenAddedValue ? Gravity.CENTER_HORIZONTAL : Gravity.CENTER}"
android:orientation="@{hasProfileEverBeenAddedValue ? LinearLayout.VERTICAL : LinearLayout.HORIZONTAL}"
android:paddingEnd="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_end_profile_not_added}"
android:paddingStart="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_start_profile_not_added}"
android:paddingEnd="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_end_profile_not_added}"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_add_view_margin_top_profile_not_added}"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
tools:ignore="RtlSymmetry">

<com.google.android.material.imageview.ShapeableImageView
android:id="@+id/profile_add_button"
android:layout_width="108dp"
android:layout_height="108dp"
app:srcCompat="@{@drawable/ic_add_profile}"
app:strokeWidth="1dp"
app:strokeColor="@color/component_color_shared_divider_color"
app:shapeAppearanceOverlay="@style/ShapeAppearanceOverlay.RoundedShape"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_chooser_add_view_circular_image_margin_top_profile_already_added : @dimen/space_0dp}"
app:strokeColor="@color/component_color_profile_chooser_activity_avatar_border_color"
app:strokeWidth="1dp" />
app:srcCompat="@{@drawable/ic_add_profile}" />

<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="@{hasProfileEverBeenAddedValue ? Gravity.CENTER_HORIZONTAL : Gravity.CENTER_VERTICAL}"
android:orientation="vertical"
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_view_already_added_description_parent_margin_top : @dimen/space_0dp}"
android:paddingStart="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_description_margin_start_profile_not_added}"
android:paddingEnd="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_description_margin_end_profile_not_added}"
android:paddingStart="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp : @dimen/profile_chooser_description_margin_start_profile_not_added}">
app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_view_already_added_description_parent_margin_top : @dimen/space_0dp}">

<TextView
android:id="@+id/add_profile_text"
style="@style/TextViewStart"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:fontFamily="sans-serif-medium"
style="@style/Caption"
android:text="@{hasProfileEverBeenAddedValue ? @string/profile_chooser_add : @string/set_up_multiple_profiles}"
android:textColor="@color/component_color_profile_chooser_activity_white_text_color"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="20sp" />

<TextView
android:id="@+id/add_profile_description_text"
style="@style/TextViewCenter"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
style="@style/TextViewStart"
android:layout_gravity="center"
android:layout_marginTop="4dp"
android:fontFamily="sans-serif"
android:text="@string/set_up_multiple_profiles_description"
android:textColor="@color/component_color_profile_chooser_activity_white_text_color"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="16sp"
android:visibility="@{hasProfileEverBeenAddedValue ? View.GONE : View.VISIBLE}" />
</LinearLayout>
</LinearLayout>
</LinearLayout>
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
Loading

0 comments on commit 2dfad61

Please sign in to comment.