Feature/issue550 add equalable content provider #766
Conversation
|
Actually, this PR is still having a little issue, which is that if you type "team", some speakers name "t" still highlighted. |
|
|
||
| import java.util.Arrays | ||
|
|
||
| interface EqualableContentsProvider { |
There was a problem hiding this comment.
Can you move this interface to androidcomponent module? for sharing 🙏
There was a problem hiding this comment.
sure, item folder would be good?
| if (speaker != other.speaker) return false | ||
| if (query != other.query) return false | ||
| override fun isTextHighlightNeedUpdate() = query?.let { | ||
| speaker.name.toLowerCase().contains(it.toLowerCase().toRegex()) |
There was a problem hiding this comment.
Sorry, this toRegex() is not good 🙇♂️
Type + crashes app, I will fix it.
|
I fixed a small issue which I mentioned, so I will push it. |
check old text highlight to update item which unnecessary highlighted remove toRegex() because it crushes when '+' is passed
Apk comparision results
Generated by 🚫 Danger |
|
Asserted successfully. 💯 Generated by 🚫 Danger |
|
|
||
| if (speaker != other.speaker) return false | ||
| if (query != other.query) return false | ||
| override fun isTextHighlightNeedUpdate() = query?.let { |
There was a problem hiding this comment.
ちょっと日本語で失礼します。 🙇
isTextHighlightNeedUpdateだと問題があるかもしれません。なぜなら例えばandroidとか入れている時に、1つのセッションをfavoriteするとマッチしている他のセッションも全て点滅してしまいます。
個人的にはproviderEqualableContentsでヒットしているときのみqueryを入れるのがバグがなくて一番良いと思っていますがどうでしょうか？
There was a problem hiding this comment.
すみません、最初にissueでコメントして頂いた意図を汲みきれていなかったみたいですね。
providerEqualableContentsにqueryを含めることでequalにならない様にする、というところまで読み切れていませんでした。
確かにチェックする箇所を1箇所にまとめる方がバグの混入が防げて良さそうですね。
修正されたコードで軽く動作確認しましたが、入力や削除時の動作で特に気になる点はなかったので問題ないかと思います 🙇♂️
…e_content_provider [Based on #766] add equalable content provider
Issue
Overview (Required)
Links
Screenshot