Switch to new relations format and show edits #420
Conversation
deepbluev7
changed the title
deepbluev7
changed the title
deepbluev7
force-pushed
the
render-edits
branch
from
58b340a
to
3556899
Compare
The UI still looks ugly, but I have no good idea atm. fixes #134
deepbluev7
force-pushed
the
render-edits
branch
from
ae59ed3
to
6e2ae1d
Compare
Does not fix the read status yet, for that we need to compare read receipts for all events after the last visible event.
| // Height of child, plus margins, plus border | ||
| implicitHeight: replyPreview.height + 10 | ||
| implicitHeight: (room && room.reply ? replyPreview.height : closeEditButton.height) + 10 |
There was a problem hiding this comment.
I think room && is probably superfluous here? Since the popup is only visible if 'room' is defined, I'm thinking the height doesn't really matter much.
There was a problem hiding this comment.
If you don't do the room &&, you get null warnings. It's stupid, but sadly necessary. It's just in the condition to prevent the null warning, apart from this the conditional sets this to reply height or edit height, depending o. if this has a reply or not.
resources/qml/ReplyPopup.qml
Outdated
| anchors.topMargin: 10 | ||
| anchors.top: parent.top | ||
| //height: 16 | ||
| text: qsTr("Abort edit") |
There was a problem hiding this comment.
Is 'Abort edit' an official phrase from somewhere in the spec? Otherwise, I'd probably use 'Cancel edit' instead. 'Abort' has a sort of negative connotation associated with it.
| ImageButton { | ||
| id: editButton | ||
|
|
||
| visible: (Settings.buttonsInTimeline && model.isEditable) || model.isEdited |
There was a problem hiding this comment.
If I'm reading this correctly, you're overloading this button to be for messages that are already edited as well as for messages that can be edited. If this is the case, it seems like it could be confusing to users. Also, what happens if someone tries to edit the same message twice? the button will say 'Edited' when the user may want to edit again and it may lead them to think they cannot edit the message again despite a button being there that allows them to edit in other scenarios.
There was a problem hiding this comment.
Well, I don't really have a good idea on how to solve that. So I just left it like that for now and will later move the edit button into a dedicated action button bar, that shows on hover, while only status icons like read, edited and timestamp are shown by default?
| lmdb::dbi_put( | ||
| txn, relationsDb, lmdb::val(relates_to), event_id); | ||
| auto relations = mtx::accessors::relations(e); | ||
| if (!relations.relations.empty()) { |
There was a problem hiding this comment.
This new code looks much nicer IMO =D
| }); | ||
| mtx::responses::utils::parse_room_account_data_events(j, events); | ||
| if (events.size() == 1) | ||
| return events.front(); |
There was a problem hiding this comment.
What happens when there's more than one event? Seems like we ignore it. But shouldn't we still return the most recent one anyway? Or otherwise somehow handle all of the events?
There was a problem hiding this comment.
This is actually an ugly hack yo reuse the parser for a list of events to parse a single event. Which is why the event is inserted into a json array at first. One day we should clean that up everywhere in our code :D
| @@ -66,7 +66,8 @@ class EventStore : public QObject | |||
| // relatedFetched event | |||
| mtx::events::collections::TimelineEvents *get(std::string_view id, | |||
| std::string_view related_to, | |||
| bool decrypt = true); | |||
| bool decrypt = true, | |||
| bool resolve_edits = true); | |||
| // always returns a proper event as long as the idx is valid | |||
| mtx::events::collections::TimelineEvents *get(int idx, bool decrypt = true); | |||
There was a problem hiding this comment.
Does this function need to have the resolve_edits changes incorporated as well?
There was a problem hiding this comment.
Not really, imo. This always resolves the edits, the other function just alloes side stepping that, to prevent loops as wel es show the edit history in the future.
| @@ -518,6 +560,8 @@ InputBar::showPreview(const QMimeData &source, QString path, const QStringList & | |||
| [this](const QByteArray data, const QString &mime, const QString &fn) { | |||
| setUploading(true); | |||
|
|
|||
| setText(""); | |||
There was a problem hiding this comment.
What if you want to send text with your upload!
There was a problem hiding this comment.
We don't support that yet. When we do that, it will need more fundamental changes :3
fixes #402
fixes #134
Currently still incomplete, as there is no indication, that a message was edited.