Skip to content

Conversation

@jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Oct 28, 2021

Description

One Line Summary

This is a back port to 3.x.x for the following two fixes which have been pulled into this branch with git cherry-pick.


This change is Reviewable

The notification background image would not show if the app has
`android:supportsRtl="true"` the `AndroidManifest.xml` AND the device
has a RTL language set. This would result in the notification becoming
a sold color (all known instances have been a white background) and only
 the title and body text showing.

Setting `android:layoutDirection="ltr"` ensures the layout, only used
for images, isn't influenced by the language direction.

This does NOT affect most notifications, only those that use
`android_background_layout` in their notification payload.

This change does NOT affect the rending of the title or body text in any
way.
Render OneSignal's custom background image notfications in
RTL (right-to-left) for those devices that have set a language
that renders in this direction.

This does not account for some spacing issues however the next commit
will cover this.
Correct the title and body 4dp padding from "left" to "start"
to support both RTL and LTR languages. This panding is important so
there is a bit of room between the large icon or the edge of the screen
depending on if the text is LTR or RTL, in that order .

Changed `layout_marginStart` to `left` since the SDK does not support
a different image based on if the device is RTL or LTR. Along with this
change the documenation should instruct developers to always put
their app icon on the left side.
@jkasten2 jkasten2 requested review from a team, Jeasmine, adamschlesinger and nan-li October 28, 2021 20:37
@jkasten2 jkasten2 merged commit 4146708 into sdk_3.X.X Oct 28, 2021
@jkasten2 jkasten2 deleted the fix/notification_background_image_rtl_3.x.x branch October 28, 2021 20:47
@jkasten2 jkasten2 mentioned this pull request Oct 28, 2021
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.

3 participants