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.

Sign up for GitHub

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-15230 Fix suppressed long method #391

Merged
merged 3 commits into from
Sep 26, 2024
Merged

ANDROID-15230 Fix suppressed long method #391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fabf224
ANDROID-15230 Fix suppressed long method
jeprubio Sep 25, 2024
20f8aba
ANDROID-15230 remove unneded experimental annotations
jeprubio Sep 25, 2024
6e42b22
ANDROID-15230 split init in private methods
jeprubio Sep 26, 2024
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
16 changes: 10 additions & 6 deletions ...rc/main/java/com/telefonica/mistica/catalog/ui/classic/components/ListsCatalogFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class ListsCatalogFragment : Fragment() {


@SuppressLint("SetTextI18n")
@Suppress("LongMethod", "CyclomaticComplexMethod")
@Suppress("CyclomaticComplexMethod")
private fun ListRowView.configureView(
withLongTitle: Boolean = false,
withTitleMaxLines: Int? = null,
Expand Down Expand Up @@ -249,11 +249,7 @@ class ListsCatalogFragment : Fragment() {
isClickable = false
}

when {
withBadge -> setBadge(true, withBadgeDescription)
withBadgeNumeric > 0 -> setNumericBadge(withBadgeNumeric, withBadgeDescription)
else -> setBadge(false, withBadgeDescription)
}
setBadge(withBadge, withBadgeDescription, withBadgeNumeric)
}

private fun getAssetResource(withAsset: Boolean, @AssetType withAssetType: Int): Int? =
Expand All @@ -271,6 +267,14 @@ class ListsCatalogFragment : Fragment() {
} else {
null
}

private fun ListRowView.setBadge(withBadge: Boolean, withBadgeDescription: String?, withBadgeNumeric: Int) {
when {
withBadge -> setBadge(true, withBadgeDescription)
withBadgeNumeric > 0 -> setNumericBadge(withBadgeNumeric, withBadgeDescription)
else -> setBadge(false, withBadgeDescription)
}
}
}

class ListViewHolder(val rowView: ListRowView) : ViewHolder(rowView)
Expand Down
141 changes: 82 additions & 59 deletions library/src/main/java/com/telefonica/mistica/button/ProgressButton.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,72 @@ class ProgressButton : FrameLayout {
}

@SuppressLint("ClickableViewAccessibility")
@Suppress("LongMethod")
private fun init(attrs: AttributeSet? = null, defStyleAttr: Int = 0) {
setupAttributes(attrs, defStyleAttr)
setupViewProperties()
setupButtonNormal()
setupButtonLoading()
setupProgressBar()
setButtonBackground()
addViews()
setVisibilityAndColors()
Comment on lines +70 to +77
Copy link
Contributor Author

@jeprubio jeprubio Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Code split in private methods

}

fun getText(): CharSequence =
buttonBackground.text

fun setText(text: CharSequence) {
buttonNormal.text = text
if (!isLoading) {
buttonBackground.text = text
}
}

fun setText(@StringRes textId: Int) {
setText(context.getString(textId))
}

fun setLoadingText(text: CharSequence) {
buttonLoading.text = text
if (isLoading) {
buttonBackground.text = text
}
}

override fun setOnClickListener(l: OnClickListener?) {
buttonBackground.setOnClickListener(l)
}

override fun setEnabled(enabled: Boolean) {
super.setEnabled(enabled)
setAlpha(enabled)
buttonBackground.isEnabled = enabled
buttonNormal.isEnabled = enabled
buttonLoading.isEnabled = enabled
progressBar.isEnabled = enabled
}

fun setIsLoading(loading: Boolean) {
if (loading) {
showLoading()
} else {
hideLoading()
}
}

fun showLoading() {
if (!isLoading) {
switchState()
}
}

fun hideLoading() {
if (isLoading) {
switchState()
}
}

private fun setupAttributes(attrs: AttributeSet?, defStyleAttr: Int) {
if (attrs != null) {
val theme = context.theme
val styledAttrs =
Expand All @@ -78,13 +142,17 @@ class ProgressButton : FrameLayout {
styledAttrs.recycle()
}
}
}

private fun setupViewProperties() {
this.importantForAccessibility = IMPORTANT_FOR_ACCESSIBILITY_NO
isClickable = false
setPadding(0, 0, 0, 0)
setBackgroundColor(Color.TRANSPARENT)
originalTextColors = buttonNormal.textColors
}

private fun setupButtonNormal() {
buttonNormal.apply {
id = NO_ID
importantForAccessibility = IMPORTANT_FOR_ACCESSIBILITY_NO
Expand All @@ -94,7 +162,9 @@ class ProgressButton : FrameLayout {
)
showTextOnly()
}
}

private fun setupButtonLoading() {
buttonLoading.apply {
id = NO_ID
importantForAccessibility = IMPORTANT_FOR_ACCESSIBILITY_NO
Expand All @@ -106,7 +176,9 @@ class ProgressButton : FrameLayout {
icon = AppCompatResources.getDrawable(context, R.drawable.empty_material_button_icon)
text = ""
}
}

private fun setupProgressBar() {
progressBar.apply {
layoutParams = LayoutParams(buttonLoading.iconSize, buttonLoading.iconSize)
.apply {
Expand All @@ -115,9 +187,11 @@ class ProgressButton : FrameLayout {
importantForAccessibility = IMPORTANT_FOR_ACCESSIBILITY_NO
isIndeterminate = true
indeterminateTintMode = PorterDuff.Mode.SRC_IN
visibility = View.INVISIBLE
visibility = INVISIBLE
}
}

private fun setButtonBackground() {
buttonBackground.apply {
id = NO_ID
importantForAccessibility = IMPORTANT_FOR_ACCESSIBILITY_YES
Expand All @@ -127,88 +201,37 @@ class ProgressButton : FrameLayout {
)
icon = null
setTextColor(Color.TRANSPARENT)
visibility = View.VISIBLE
visibility = VISIBLE
setOnTouchListener { view, event ->
when (event.action) {
MotionEvent.ACTION_DOWN -> {
buttonNormal.isPressed = true
buttonLoading.isPressed = true
}

MotionEvent.ACTION_MOVE -> {
val outsideBounds =
event.x < 0 || event.y < 0 || event.x > view.measuredWidth || event.y > view.measuredHeight
buttonNormal.isPressed = !outsideBounds
buttonLoading.isPressed = !outsideBounds
}

MotionEvent.ACTION_UP, MotionEvent.ACTION_CANCEL -> {
buttonNormal.isPressed = false
buttonLoading.isPressed = false
view.performClick()
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this line needed? was it being handled in other side?

Copy link
Contributor Author

@jeprubio jeprubio Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Detekt marks it as error if view.performClick() is not invoked on a setOnTouchListener lambda

onTouch lambda should call View#performClick when a click is detected

Explanation for issues of type "ClickableViewAccessibility":
If a View that overrides onTouchEvent or uses an OnTouchListener does not also implement performClick and call it when clicks are detected, the View may not handle accessibility actions properly. Logic handling the click actions should ideally be placed in View#performClick as some accessibility services invoke performClick when a click action should occur.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm worried about any unexpected behavior and if it wasn't added before for a reason.. but I guess the button is still working as before right?

Copy link
Contributor Author

@jeprubio jeprubio Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yup, I made a video in the pr description which checks the 2 updated elements (the progress button and the list in compose)

}
}
false
}
}
}

private fun addViews() {
addView(buttonBackground)
addView(buttonNormal)
addView(buttonLoading)
addView(progressBar)

setVisibilityAndColors()
}

fun getText(): CharSequence =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved the public methods on top of the private ones I've created

buttonBackground.text

fun setText(text: CharSequence) {
buttonNormal.text = text
if (!isLoading) {
buttonBackground.text = text
}
}

fun setText(@StringRes textId: Int) {
setText(context.getString(textId))
}

fun setLoadingText(text: CharSequence) {
buttonLoading.text = text
if (isLoading) {
buttonBackground.text = text
}
}

override fun setOnClickListener(l: OnClickListener?) {
buttonBackground.setOnClickListener(l)
}

override fun setEnabled(enabled: Boolean) {
super.setEnabled(enabled)
setAlpha(enabled)
buttonBackground.isEnabled = enabled
buttonNormal.isEnabled = enabled
buttonLoading.isEnabled = enabled
progressBar.isEnabled = enabled
}

fun setIsLoading(loading: Boolean) {
if (loading) {
showLoading()
} else {
hideLoading()
}
}

fun showLoading() {
if (!isLoading) {
switchState()
}
}

fun hideLoading() {
if (isLoading) {
switchState()
}
}

private fun switchState() {
Expand Down
Loading
Loading