Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

Display stickers #247

Merged
merged 4 commits into from
Mar 29, 2018
Merged

Display stickers #247

merged 4 commits into from
Mar 29, 2018

Conversation

Tyuoli
Copy link
Contributor

@Tyuoli Tyuoli commented Mar 28, 2018

@Tyuoli Tyuoli requested review from manuroe and giomfo March 28, 2018 12:34
@ylecollen
Copy link
Contributor

ylecollen commented Mar 28, 2018

StickerInfo and StickerMessage are c+ped from ImageMessage and ImageInfo.

StickerMessage could inherit from imageMessage like this


_public class StickerMessage extends ImageMessage {

// add custom fields if needed

 public StickerMessage () {
    msgtype = MSGTYPE_STICKER; <--- you forgot this one
}

}

StickerInfo becomes useless.

Moreover, it would simply a lot your UI code which c+p the imageMessage code (in the MediasHelpers) to manage StickerMessages .

StickerMessage seems being a custom ImageMessage.

@Tyuoli
Copy link
Contributor Author

Tyuoli commented Mar 29, 2018

m.sticker isn't a new message type but a new event type. I will try to create a new StickerMessageModel to parse sticker json and inherit of ImageMessage.

} else if (TextUtils.equals(Event.EVENT_TYPE_STICKER, event.getType())) {
StickerMessage stickerMessage = JsonUtils.toStickerMessage(event.getContent());

if (stickerMessage.isThumbnailLocalContent()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As StickerMessage is now a MediaMessage, could we merge this C+P code with the code above?

@@ -26,6 +26,10 @@
public static final String MSGTYPE_FILE = "m.file";
public static final String FORMAT_MATRIX_HTML = "org.matrix.custom.html";

// Add, in local, a fake message type in order to StickerMessage can inherit Message class
// Because sticker isn't a new message type but a new event type without msgtype field
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove "new" in the comment?
Stickers will be no more new next time we play(code) with them.

@@ -106,6 +107,8 @@ public CharSequence getTextualDisplay() {
public CharSequence getTextualDisplay(Integer displayNameColor) {
CharSequence text = null;

Log.e(LOG_TAG, "#getTextualDisplay : " + mEvent.eventId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this dev log.

*/
package org.matrix.androidsdk.rest.model.message;

public class StickerJsonMessage {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we have a comment for this class?
Explaining that it is just here as an intermediate object to create a StickerMessage from a m.sticker event type.

Remove dev log
@@ -727,7 +729,7 @@ public void removeMediasBefore(final Context context, final long timestamp) {
if (null != events) {
for (Event event : events) {
try {
if (TextUtils.equals(Event.EVENT_TYPE_MESSAGE, event.getType())) {
if (TextUtils.equals(Event.EVENT_TYPE_MESSAGE, event.getType()) || TextUtils.equals(Event.EVENT_TYPE_STICKER, event.getType())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it work?

@Tyuoli Tyuoli merged commit 128e6ef into develop Mar 29, 2018
@Tyuoli Tyuoli deleted the sdk_stickers branch March 29, 2018 12:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants