-
Notifications
You must be signed in to change notification settings - Fork 676
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
Hide "X made no changes" event by default in timeline (#1430) #1436
Conversation
...android/src/main/java/im/vector/matrix/android/api/session/room/timeline/TimelineSettings.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some remarks
@@ -38,6 +39,7 @@ internal object EventMapper { | |||
eventEntity.content = ContentMapper.map(event.content) | |||
val resolvedPrevContent = event.prevContent ?: event.unsignedData?.prevContent | |||
eventEntity.prevContent = ContentMapper.map(resolvedPrevContent) | |||
eventEntity.isUseless = event.type == EventType.STATE_ROOM_MEMBER && eventEntity.content == eventEntity.prevContent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it should be saved in db?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's saved in DB to be able to do query on it. Maybe you known a better way to do query which check if two value in the DB are identical?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right, thanks to realm for not being flexible enough...
Could you create a class like IsUselessResolver to handle this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it, but, it will call again what is called in the three line above, so for the perf, as we are inserting event in DB here, also from initial sync, I do not know if it's a good idea... What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok done, I let you check
Fixes #1430
Before PR:
After PR: