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

ANDROID-13311 Evolve Feedback Screen #300

Merged
merged 6 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
android:id="@+id/sample_icon"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:src="@drawable/feedback_info_telefonica"
android:src="@drawable/feedback_info"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintLeft_toLeftOf="parent"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,11 @@ class FeedbackScreenView : ConstraintLayout {
private var firstButtonClickListener: OnClickListener? = null
private var secondButtonClickListener: OnClickListener? = null

@RawRes private var customAnimation: Int? = null
@DrawableRes private var customIcon: Int? = null
@RawRes
private var customAnimation: Int? = null

@DrawableRes
private var customIcon: Int? = null

constructor(context: Context) : super(context) {
init(context)
Expand Down Expand Up @@ -337,17 +340,12 @@ class FeedbackScreenView : ConstraintLayout {
else -> icon.visibility = View.GONE
}
isIconAnimated = animationResource != null
icon.addOnLayoutChangeListener { _, _, _, _, _, _, _, _, _ ->
icon.translationX = -(icon.width - context.convertDpToPx(64).toFloat()) / 2
}
}

private fun configureTexts() {
@ColorInt val titleTextColor: Int =
context.getThemeColor(if (isInversePresentation()) R.attr.colorTextPrimaryInverse else R.attr.colorTextPrimary)
@ColorInt val titleTextColor: Int = context.getThemeColor(if (isInversePresentation()) R.attr.colorTextPrimaryInverse else R.attr.colorTextPrimary)
title.setTextColor(titleTextColor)
@ColorInt val subtitleTextColor: Int =
context.getThemeColor(if (isInversePresentation()) R.attr.colorTextPrimaryInverse else R.attr.colorTextSecondary)
@ColorInt val subtitleTextColor: Int = context.getThemeColor(if (isInversePresentation()) R.attr.colorTextPrimaryInverse else R.attr.colorTextSecondary)
subtitle.setTextColor(subtitleTextColor)
setFeedbackTitle(titleText)
setFeedbackSubtitle(subtitleText)
Expand Down Expand Up @@ -395,29 +393,46 @@ class FeedbackScreenView : ConstraintLayout {

fun animateViews() {
if (isIconAnimated) {
val animation = AnimatorSet().apply {
val titleAnimation = AnimatorSet().apply {
playTogether(
getFadeInAnim(title),
getTranslationYAnim(title)
)
interpolator = getCubicBezierInterpolator()
duration = TEXTS_ANIMATION_DURATION
startDelay = TITLE_ANIMATION_DELAY
}
val subtitleAnimation = AnimatorSet().apply {
playTogether(
getFadeInAnim(subtitle),
getTranslationYAnim(subtitle)
)
interpolator = getCubicBezierInterpolator()
duration = TEXTS_ANIMATION_DURATION
startDelay = SUBTITLE_ANIMATION_DELAY
}

val extraAnimation = AnimatorSet().apply {
playTogether(
getFadeInAnim(customContentContainer),
getFadeInAnim(errorReference),
getTranslationYAnim(title),
getTranslationYAnim(subtitle),
getTranslationYAnim(customContentContainer),
getFadeInAnim(errorReference),
getTranslationYAnim(errorReference),
)
interpolator = getCubicBezierInterpolator()
duration = TEXTS_ANIMATION_DURATION
startDelay = TEXTS_ANIMATION_DELAY
startDelay = if (subtitleText.isEmpty()) SUBTITLE_ANIMATION_DELAY else EXTRAS_ANIMATION_DELAY
}

title.alpha = 0F
subtitle.alpha = 0F
errorReference.alpha = 0F
customContentContainer.alpha = 0F

animation.start()
icon.playAnimation()
titleAnimation.start()
subtitleAnimation.start()
extraAnimation.start()
executeHapticFeedback()
}
}
Expand Down Expand Up @@ -462,7 +477,9 @@ class FeedbackScreenView : ConstraintLayout {
const val TYPE_INFO = 2

const val TEXTS_ANIMATION_DURATION = 800L
const val TEXTS_ANIMATION_DELAY = 200L
const val TITLE_ANIMATION_DELAY = 600L
const val SUBTITLE_ANIMATION_DELAY = 900L
const val EXTRAS_ANIMATION_DELAY = 1_200L

const val HAPTIC_FEEDBACK_DEFAULT_DELAY = 450L
const val HAPTIC_FEEDBACK_ERROR_DELAY = 500L
Expand Down
10 changes: 0 additions & 10 deletions library/src/main/res/drawable/feedback_info_telefonica.xml

This file was deleted.

25 changes: 8 additions & 17 deletions library/src/main/res/layout/screen_feedback.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,15 @@
android:layout_height="wrap_content"
android:focusable="true"
android:orientation="vertical"
android:paddingTop="24dp" >
android:paddingTop="64dp"
android:layout_marginStart="16dp"
android:layout_marginEnd="16dp">

<com.airbnb.lottie.LottieAnimationView
android:id="@+id/icon"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="24dp"
android:layout_marginTop="24dp"
android:scaleType="center"
app:layout_constraintStart_toStartOf="parent"
android:layout_height="48dp"

Choose a reason for hiding this comment

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

I understand that the icon is at 48, and that's correct, but even with the internal margin we discussed earlier, it still appears visually too small. Is it possible for the icon to scale according to the screen density?
61112349-2b1d-45fc-a3ed-385c4bd43600

Copy link
Contributor

@jeprubio jeprubio Sep 14, 2023

Choose a reason for hiding this comment

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

That 48dp, the dp in there means density-independent pixels so it's scaling according to screen density. I suspect the feedback_success_vivo_new.json has some margins in it so then when the height are set to 48dp it has to be shrunk to fit in there, I would check that.

android:scaleType="fitStart"
app:lottie_enableMergePathsForKitKatAndAbove="true"
app:lottie_progress="0" />

Expand All @@ -32,36 +31,28 @@
style="@style/AppTheme.TextAppearance.Preset6"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="24dp"
android:layout_marginTop="24dp"
android:layout_marginEnd="24dp" />
android:layout_marginTop="24dp" />

<TextView
android:id="@+id/subtitle"
style="@style/AppTheme.TextAppearance.Preset3"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="24dp"
android:layout_marginTop="16dp"
android:layout_marginEnd="24dp" />
android:layout_marginTop="16dp" />

<FrameLayout
android:id="@+id/custom_content"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="24dp"
android:layout_marginEnd="24dp"
android:layout_marginTop="24dp"
android:layout_marginTop="16dp"
android:visibility="gone" />

<TextView
android:id="@+id/error_reference"
style="@style/AppTheme.TextAppearance.Preset2"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="24dp"
android:layout_marginTop="16dp"
android:layout_marginEnd="24dp"
android:textColor="?colorTextSecondary"
android:visibility="gone" />

Expand Down
Loading