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

Feature/support rtl #606

Merged
merged 11 commits into from Feb 5, 2018
Merged

Conversation

atsuko-fukui
Copy link
Contributor

Issue

Overview (Required)

  • Support RTL layout
  • Some of screen (see session detail screen shot blow) looks ugly because Japanese and English are NOT RTL language. Should I set android:textDirection="locale" or marginEnd as well as marginStart?

Screenshot

Set RTL On

Screen Before After
Feed
Session detail
About this app
Sponsors

@KeithYokoma KeithYokoma self-requested a review February 5, 2018 05:29
@KeithYokoma
Copy link
Contributor

👁

@atsuko-fukui
Copy link
Contributor Author

I missed your reply 😮

@KeithYokoma
Copy link
Contributor

np

Copy link
Contributor

@KeithYokoma KeithYokoma left a comment

Choose a reason for hiding this comment

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

TextView has a property for text alignment.

android:textAlignment

To align the text according to layout direction, use viewStart or viewEnd:
viewStart aligns text to start of the view: Left alignment for LTR layout direction and right alignment for RTL layout direction. viewEnd behaves opposite way.

@atsuko-fukui
Copy link
Contributor Author

Looks good with android:textAlignment 😎

text = when (roomName) { null -> ""
else -> context.getString(R.string.room_format, prefix, roomName)
roomName ?: return
val isLayoutDirectionRtl = resources.configuration.layoutDirection == View.LAYOUT_DIRECTION_RTL
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you extract this comparison as an extension method of View?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done with 5937050

@KeithYokoma
Copy link
Contributor

💥Nice! 💥

@atsuko-fukui
Copy link
Contributor Author

Also I add android:autoMirrored to directional symbols. Let me know if I missed some directional icons.

e.g.) Map icon and people icon are changed:

setPadding(start, top, end, bottom)
}

fun View.isLayoutDirectionRtl() =
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -53,7 +53,7 @@ open class BottomNavigationBehavior : CoordinatorLayout.Behavior<BottomNavigatio
internal fun updateSnackBarPaddingBottomByBottomNavigationView(view: BottomNavigationView) {
snackbar?.apply {
val translateY = (view.height - view.translationY).toInt()
setPadding(paddingLeft, paddingTop, paddingRight, translateY)
setPadding(paddingEnd, paddingTop, paddingStart, translateY)
Copy link
Contributor

Choose a reason for hiding this comment

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

👀 I guess here should be setPaddingWithLayoutDirction(paddingStart, paddingTop, paddingEnd, translateY)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😂

@@ -34,3 +34,13 @@ var View.elevationForPostLollipop: Float
elevation = value
}
}

fun View.setPaddingWithLayoutDirction(start: Int, top: Int, end: Int, bottom: Int) =
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

android:layout_marginTop="16dp"
android:fontFamily="sans-serif-light"
android:text="@string/about_what_is_prefix"
android:textColor="#de000000"
android:textSize="23sp"
android:textStyle="normal"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintLeft_toLeftOf="@id/conference_name"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

app:highlightText="@{searchQuery}"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintHorizontal_bias="0.0"
app:layout_constraintStart_toEndOf="@id/speaker_image"
Copy link
Contributor

Choose a reason for hiding this comment

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

👏 nice catch

@@ -15,8 +15,10 @@ import android.text.style.StyleSpan
import android.view.ActionMode
import android.view.Menu
import android.view.MenuItem
import android.view.View

Choose a reason for hiding this comment

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

⚠️ Unused import

Copy link
Contributor

@jmatsu jmatsu left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@KeithYokoma KeithYokoma left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@atsuko-fukui
Copy link
Contributor Author

@jmatsu Thanks! Done with b870783

Copy link
Contributor

@jmatsu jmatsu left a comment

Choose a reason for hiding this comment

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

Great!
Thank you so much!

@jmatsu jmatsu merged commit 451042a into DroidKaigi:master Feb 5, 2018
@atsuko-fukui atsuko-fukui deleted the feature/support_rtl branch February 5, 2018 07:53
@takahirom takahirom added this to the 1.1.0(〜02/05) milestone Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support RTL
5 participants