Safe epoxy char sequence #4837
Safe epoxy char sequence #4837
Conversation
…utation of the CharSequence, which can happen during rendering
Or use String if possible
bmarty
force-pushed
the
feature/bma/safe_epoxy_char_sequence
branch
from
9c602a7
to
4642299
Compare
| private val hash = charSequence.toString().hashCode() | ||
|
|
||
| override fun hashCode() = hash | ||
| override fun equals(other: Any?) = other is EpoxyCharSequence && other.hash == hash |
There was a problem hiding this comment.
We do not compare the String content, since the risk of collision is low enough, and Epoxy is mainly using hashCode() to compare items
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright (c) 2021 New Vector Ltd | |||
| * Copyright (c) 2022 New Vector Ltd | |||
| @@ -33,7 +33,7 @@ import im.vector.app.core.extensions.setAttributeTintedImageResource | |||
| abstract class RadioButtonItem : VectorEpoxyModel<RadioButtonItem.Holder>() { | |||
|
|
|||
| @EpoxyAttribute | |||
| var title: CharSequence? = null | |||
| var title: String? = null | |||
There was a problem hiding this comment.
Whenever a CharSequence is not required, it simpler to use an immutable String
| /** | ||
| * Wrapper for a CharSequence, which support mutation of the CharSequence, which can happen during rendering | ||
| */ | ||
| class EpoxyCharSequence(val charSequence: CharSequence) { |
There was a problem hiding this comment.
Maybe let the EpoxyCharSequence inherits CharSequence like this, to avoid accessing charSequence value everywhere
class EpoxyCharSequence(val charSequence: CharSequence) : CharSequence by charSequence
There was a problem hiding this comment.
It does not work, the TextView displays the Object and not the CharSequence... Weird
There was a problem hiding this comment.
According to @ganfra investigation, it can work using
class EpoxyCharSequence(val charSequence: CharSequence) : Spannable by SpannableString(charSequence) {...}but I prefer not to create a new SpannableString for each charSequence.
Using .charSequence whenever it is necesssary is probably better, because it forces the developer to do it (no "hidden magic")
Using CharSequence in Epoxy item is not safe, since CharSequence are no immutable.
This PR introduce a wrapper for CharSequences used in Epoxy. If CharSequence is not mandatory, use immutable String.