Skip to content
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

Fix#2140: Fix username's cutoff in Home Page #2396

Merged
merged 27 commits into from
Jan 21, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a0e17ec
Add flexBoxLayout to welcome.xml
prayutsu Jan 2, 2021
f90fc6f
Add a comma after the newly added dependency
prayutsu Jan 5, 2021
90bab24
Maintain the alphabetical order of the dependencies in build.gradle(:…
prayutsu Jan 5, 2021
227e06c
Add flexboxlayout artifact in the correct section
prayutsu Jan 6, 2021
9e80d24
Fix issue for portrait layout phones
prayutsu Jan 11, 2021
476537b
Make changes in all the files
prayutsu Jan 13, 2021
01a7ddc
Fix nit changes
prayutsu Jan 14, 2021
bb09d37
Add first test case
prayutsu Jan 14, 2021
90c78be
Add all the test cases
prayutsu Jan 14, 2021
dd3243c
Add all the test cases and uncomment all other test cases
prayutsu Jan 14, 2021
ef013ce
Resolve merge conflicts
prayutsu Jan 14, 2021
4df8572
Fix unwanted changes
prayutsu Jan 14, 2021
6e8f5f3
Fix unwanted changes
prayutsu Jan 14, 2021
cd3cd92
Restore HomeActivityTest.kt
prayutsu Jan 14, 2021
623bf07
Remove Unwanted Changes
prayutsu Jan 14, 2021
b17a1d9
Remove Unwanted Changes
prayutsu Jan 14, 2021
fe07e68
Make a separate function to initialize Long Name Profile to fix faili…
prayutsu Jan 14, 2021
1ee8b2a
Add @RunOn(ESPRESSO) notation to the new test cases
prayutsu Jan 14, 2021
62e152e
Fix suggested changes
prayutsu Jan 15, 2021
24bb124
Fix test cases in other files
prayutsu Jan 15, 2021
108d3b8
Resolve Merge Conflicts
prayutsu Jan 18, 2021
5efa0a5
Merge latest develop
prayutsu Jan 20, 2021
afc25d0
Fix unwanted changes
prayutsu Jan 20, 2021
a4bc494
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Jan 20, 2021
4a13433
Fix suggested changes
prayutsu Jan 20, 2021
b2899b8
Fix nits
prayutsu Jan 20, 2021
65d6a58
Fix nits
prayutsu Jan 21, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ maven_install(
"com.crashlytics.sdk.android:crashlytics:2.9.8",
"com.github.bumptech.glide:glide:4.11.0",
"com.github.bumptech.glide:mocks:4.11.0",
"com.google.android:flexbox:2.0.1",
"com.google.android.material:material:1.2.0-alpha02",
"com.google.firebase:firebase-analytics:17.5.0",
"com.google.firebase:firebase-crashlytics:17.1.1",
Expand Down
2 changes: 2 additions & 0 deletions app/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ android_library(
artifact("androidx.viewpager:viewpager:1.0.0"),
artifact("androidx.viewpager2:viewpager2:1.0.0"),
artifact("com.chaos.view:pinview:1.4.4"),
artifact("com.google.android:flexbox:2.0.1"),
artifact("com.google.android.material:material"),
artifact("de.hdodenhof:circleimageview:3.0.1"),
artifact("javax.annotation:javax.annotation-api"),
Expand Down Expand Up @@ -625,6 +626,7 @@ kt_android_library(
artifact("androidx.viewpager2:viewpager2:1.0.0"),
artifact("androidx.work:work-runtime-ktx:2.4.0"),
artifact("com.caverock:androidsvg-aar"),
artifact("com.google.android:flexbox:2.0.1"),
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
artifact("javax.annotation:javax.annotation-api:jar"),
],
)
Expand Down
3 changes: 2 additions & 1 deletion app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ dependencies {
'androidx.work:work-runtime-ktx:2.4.0',
'com.chaos.view:pinview:1.4.4',
'com.github.bumptech.glide:glide:4.11.0',
'com.google.android:flexbox:2.0.1',
'com.google.android.material:material:1.2.0-alpha02',
'com.google.dagger:dagger:2.24',
'com.google.firebase:firebase-analytics:17.5.0',
Expand All @@ -124,7 +125,7 @@ dependencies {
"org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version",
'org.jetbrains.kotlinx:kotlinx-coroutines-core:1.2.1',
'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.2.1',
'org.mockito:mockito-core:2.7.22'
'org.mockito:mockito-core:2.7.22',
anandwana001 marked this conversation as resolved.
Show resolved Hide resolved
)
testImplementation(
'androidx.test:core:1.2.0',
Expand Down
65 changes: 35 additions & 30 deletions app/src/main/res/layout-land/welcome.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,49 @@

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">
android:layout_height="wrap_content"
android:paddingStart="72dp"
android:paddingEnd="72dp">

<TextView
android:id="@+id/welcome_text_view"
android:layout_width="0dp"
<com.google.android.flexbox.FlexboxLayout
android:id="@+id/welcome_message_container"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="72dp"
android:fontFamily="sans-serif"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"
android:text="@{viewModel.greeting}"
app:alignContent="flex_start"
app:alignItems="flex_start"
app:flexDirection="row"
app:flexWrap="wrap"
app:justifyContent="flex_start"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
app:layout_constraintTop_toTopOf="parent">

<TextView
android:id="@+id/profile_name_textview"
android:layout_width="0dp"
android:layout_height="0dp"
android:layout_marginStart="8dp"
android:layout_marginEnd="72dp"
android:ellipsize="end"
android:maxLines="1"
android:fontFamily="sans-serif"
android:text="@{String.format(@string/welcome_profile_name, viewModel.profileName)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@+id/welcome_text_view"
app:layout_constraintBottom_toBottomOf="@id/welcome_text_view"
app:layout_constraintTop_toTopOf="@+id/welcome_text_view" />
<TextView
android:id="@+id/welcome_text_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginEnd="8dp"
android:fontFamily="sans-serif"
android:text="@{viewModel.greeting}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp" />

<TextView
android:id="@+id/profile_name_textview"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:fontFamily="sans-serif"
android:text="@{String.format(@string/welcome_profile_name, viewModel.profileName)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp" />
</com.google.android.flexbox.FlexboxLayout>

<View
android:layout_width="0dp"
android:layout_width="172dp"
android:layout_height="6dp"
android:layout_marginTop="4dp"
android:background="@color/oppiaLightBrown"
app:layout_constraintEnd_toEndOf="@+id/welcome_text_view"
app:layout_constraintStart_toStartOf="@+id/welcome_text_view"
app:layout_constraintTop_toBottomOf="@+id/welcome_text_view" />
app:layout_constraintStart_toStartOf="@+id/welcome_message_container"
app:layout_constraintTop_toBottomOf="@+id/welcome_message_container" />
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
67 changes: 36 additions & 31 deletions app/src/main/res/layout-sw600dp-land/welcome.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,49 @@

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">
android:layout_height="wrap_content"
android:paddingStart="@dimen/home_tablet_land_outer_margin"
android:paddingEnd="@dimen/home_tablet_land_outer_margin">

<TextView
android:id="@+id/welcome_text_view"
android:layout_width="0dp"
<com.google.android.flexbox.FlexboxLayout
android:id="@+id/welcome_message_container"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="@dimen/home_tablet_land_outer_margin"
android:fontFamily="sans-serif"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"
android:text="@{viewModel.greeting}"
app:alignContent="flex_start"
app:alignItems="flex_start"
app:flexDirection="row"
app:flexWrap="wrap"
app:justifyContent="flex_start"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
app:layout_constraintTop_toTopOf="parent">

<TextView
android:id="@+id/profile_name_textview"
android:layout_width="0dp"
android:layout_height="0dp"
android:layout_marginStart="8dp"
android:layout_marginEnd="72dp"
android:ellipsize="end"
android:maxLines="1"
android:fontFamily="sans-serif"
android:text="@{String.format(@string/welcome_profile_name, viewModel.profileName)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@+id/welcome_text_view"
app:layout_constraintBottom_toBottomOf="@id/welcome_text_view"
app:layout_constraintTop_toTopOf="@+id/welcome_text_view" />
<TextView
android:id="@+id/welcome_text_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginEnd="8dp"
android:fontFamily="sans-serif"
android:text="@{viewModel.greeting}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp" />

<TextView
android:id="@+id/profile_name_textview"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:fontFamily="sans-serif"
android:text="@{String.format(@string/welcome_profile_name, viewModel.profileName)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp" />
</com.google.android.flexbox.FlexboxLayout>

<View
android:layout_width="0dp"
android:layout_width="172dp"
android:layout_height="6dp"
android:layout_marginTop="4dp"
android:background="@color/oppiaBrownDark"
app:layout_constraintEnd_toEndOf="@+id/welcome_text_view"
app:layout_constraintStart_toStartOf="@+id/welcome_text_view"
app:layout_constraintTop_toBottomOf="@+id/welcome_text_view" />
android:background="@color/oppiaLightBrown"
app:layout_constraintStart_toStartOf="@+id/welcome_message_container"
app:layout_constraintTop_toBottomOf="@+id/welcome_message_container" />
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
64 changes: 34 additions & 30 deletions app/src/main/res/layout-sw600dp-port/welcome.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,48 @@

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">
android:layout_height="wrap_content"
android:paddingStart="@dimen/home_tablet_port_outer_margin"
android:paddingEnd="@dimen/home_tablet_port_outer_margin">

<TextView
android:id="@+id/welcome_text_view"
android:layout_width="0dp"
<com.google.android.flexbox.FlexboxLayout
android:id="@+id/welcome_message_container"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="@dimen/home_tablet_port_outer_margin"
android:fontFamily="sans-serif"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"
android:text="@{viewModel.greeting}"
app:alignContent="flex_start"
app:alignItems="flex_start"
app:flexDirection="row"
app:flexWrap="wrap"
app:justifyContent="flex_start"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
app:layout_constraintTop_toTopOf="parent">

<TextView
android:id="@+id/profile_name_textview"
android:layout_width="0dp"
android:layout_height="0dp"
android:layout_marginStart="8dp"
android:layout_marginEnd="72dp"
android:ellipsize="end"
android:maxLines="1"
android:fontFamily="sans-serif"
android:text="@{String.format(@string/welcome_profile_name, viewModel.profileName)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@+id/welcome_text_view"
app:layout_constraintBottom_toBottomOf="@id/welcome_text_view"
app:layout_constraintTop_toTopOf="@+id/welcome_text_view" />
<TextView
android:id="@+id/welcome_text_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginEnd="8dp"
android:fontFamily="sans-serif"
android:text="@{viewModel.greeting}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"/>
<TextView
android:id="@+id/profile_name_textview"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:fontFamily="sans-serif"
android:text="@{String.format(@string/welcome_profile_name, viewModel.profileName)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp" />
</com.google.android.flexbox.FlexboxLayout>

<View
android:layout_width="0dp"
android:layout_width="172dp"
android:layout_height="6dp"
android:layout_marginTop="4dp"
android:background="@color/oppiaBrownDark"
app:layout_constraintEnd_toEndOf="@+id/welcome_text_view"
app:layout_constraintStart_toStartOf="@+id/welcome_text_view"
app:layout_constraintTop_toBottomOf="@+id/welcome_text_view" />
app:layout_constraintStart_toStartOf="@+id/welcome_message_container"
app:layout_constraintTop_toBottomOf="@+id/welcome_message_container" />
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
65 changes: 35 additions & 30 deletions app/src/main/res/layout/welcome.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,49 @@

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">
android:layout_height="wrap_content"
android:paddingStart="28dp"
android:paddingEnd="28dp">

<TextView
android:id="@+id/welcome_text_view"
android:layout_width="0dp"
<com.google.android.flexbox.FlexboxLayout
android:id="@+id/welcome_message_container"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="28dp"
android:fontFamily="sans-serif"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"
android:text="@{viewModel.greeting}"
app:alignContent="flex_start"
app:alignItems="flex_start"
app:flexDirection="row"
app:flexWrap="wrap"
app:justifyContent="flex_start"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
app:layout_constraintTop_toTopOf="parent">

<TextView
android:id="@+id/profile_name_textview"
android:layout_width="0dp"
android:layout_height="0dp"
android:layout_marginStart="8dp"
android:layout_marginEnd="28dp"
android:ellipsize="end"
android:maxLines="1"
android:fontFamily="sans-serif"
android:text="@{String.format(@string/welcome_profile_name, viewModel.profileName)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@+id/welcome_text_view"
app:layout_constraintBottom_toBottomOf="@id/welcome_text_view"
app:layout_constraintTop_toTopOf="@+id/welcome_text_view" />
<TextView
android:id="@+id/welcome_text_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginEnd="4dp"
android:fontFamily="sans-serif"
android:text="@{viewModel.greeting}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp" />

<TextView
android:id="@+id/profile_name_textview"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:fontFamily="sans-serif"
android:text="@{String.format(@string/welcome_profile_name, viewModel.profileName)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="24sp" />
</com.google.android.flexbox.FlexboxLayout>

<View
android:layout_width="0dp"
android:layout_width="172dp"
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Could we try setting this to 0dp and constraining the end to the end of the container, then making the container wrap_content & give it horizontal bias to be left-aligned in the constraint layout? I think flexbox layout is supposed to support growing up to its maximum before wrapping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But in that case, the orange line could take the full width of the screen. Do we want that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am expecting the following output if your suggestion is implemented -
If the salutation and username appear side to side, then the orange line would be as wide as the flexboxlayout and if the username appears below the salutation then the orange line would be as wide as the username's first line.

So, if we want this behaviour, I'll make changes in the next commit.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

That's up to @mschanteltc & @rt4914. My understanding about why we decided to go with a fixed size was because we couldn't make it as long as the text, but this is an approximation of that even if it might sometimes span the whole length of the view.

FWIW I think the greeting-only line length looks better in the longer name situations.

android:layout_height="6dp"
android:layout_marginTop="4dp"
android:background="@color/oppiaLightBrown"
app:layout_constraintEnd_toEndOf="@+id/welcome_text_view"
app:layout_constraintStart_toStartOf="@+id/welcome_text_view"
app:layout_constraintTop_toBottomOf="@+id/welcome_text_view" />
app:layout_constraintStart_toStartOf="@+id/welcome_message_container"
app:layout_constraintTop_toBottomOf="@+id/welcome_message_container" />
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
Loading