Skip to content

Commit aef84cf

Browse files
author
Kitsune Ral
committed
Make sure events get loaded if the timeline is empty
After fixing the race in the room switching procedure (see 4ce59f7) another race showed up, this time between the model reset and updating root.room property in QML. Because this property is distinct from messageModel.room, it gets updated later, specifically after onModelReset() is called. onModelReset() itself already uses messageModel.room instead of root.room exactly for that reason but ensurePreviousContent() called from there still operates on root.room. That alone wouldn't be much of a hurdle (ensurePreviousContent() could be rewired to messageModel.room just as well) but at the moment onModelReset() is called chatView has not created any delegates for the model yet - meaning that contentY and originY used in ensurePreviousContent() are 0 even when the model has plenty of events loaded, leading to a false-positive condition and an unneeded request to the homeserver for more messages. This commit moves requesting the initial batch of historical events to the model. It does not request a lot, therefore the turnaround is short; and that solves the timeline bootstrapping after a room switch. Eventually this might even move to libQuotient, because events have to be loaded to the room when it is displayed, regardless of the client - but that's something to ponder separately. And while we're at it, the property tracking the number of requested historical events has been moved to QuaternionRoom, anticipating its further move to libQuotient.
1 parent 25ce1c6 commit aef84cf

File tree

4 files changed

+36
-19
lines changed

4 files changed

+36
-19
lines changed

client/models/messageeventmodel.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ void MessageEventModel::changeRoom(QuaternionRoom* room)
181181
qCDebug(EVENTMODEL)
182182
<< "Event model connected to room" << room->objectName() //
183183
<< "as" << room->localUser()->id();
184+
// If the timeline isn't loaded, ask for at least something right away
185+
if (room->timelineSize() == 0)
186+
room->getHistory(30);
184187
}
185188
endResetModel();
186189
emit readMarkerUpdated();

client/qml/Timeline.qml

+11-19
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,6 @@ Page {
256256
contentHeight > 0 && count > 0 ? count / contentHeight : 0.03
257257
// 0.03 is just an arbitrary reasonable number
258258

259-
property int lastRequestedEvents: 0
260-
readonly property int currentRequestedEvents:
261-
room && room.eventsHistoryJob ? lastRequestedEvents : 0
262-
263259
property var textEditWithSelection
264260
property real readMarkerContentPos: originY
265261
readonly property real readMarkerViewportPos:
@@ -287,10 +283,8 @@ Page {
287283
// Check if we're about to bump into the ceiling in
288284
// 2 seconds and if yes, request the amount of messages
289285
// enough to scroll at this rate for 3 more seconds
290-
if (velocity > 0 && contentY - velocity*2 < originY) {
291-
lastRequestedEvents = velocity * eventDensity * 3
292-
room.getPreviousContent(lastRequestedEvents)
293-
}
286+
if (velocity > 0 && contentY - velocity*2 < originY)
287+
room.getHistory(velocity * eventDensity * 3)
294288
}
295289
onContentYChanged: ensurePreviousContent()
296290
onContentHeightChanged: ensurePreviousContent()
@@ -346,11 +340,9 @@ Page {
346340
chatView.saveViewport(true)
347341
}
348342
function onModelReset() {
349-
if (messageModel.room) {
350-
// Load events if there are not enough of them
351-
chatView.ensurePreviousContent()
352-
chatView.positionViewAtBeginning()
353-
}
343+
// NB: at this point, the actual delegates are not instantiated
344+
// yet, so defer all actions to when at least some are
345+
scrollFinisher.scrollViewTo(0, ListView.Contain)
354346
}
355347
}
356348

@@ -479,7 +471,7 @@ Page {
479471

480472
// A proxy property for animation
481473
property int requestedHistoryEventsCount:
482-
chatView.currentRequestedEvents
474+
room ? room.requestedEventsCount : 0
483475
AnimationBehavior on requestedHistoryEventsCount {
484476
NormalNumberAnimation { }
485477
}
@@ -619,8 +611,8 @@ Page {
619611
color: palette.alternateBase
620612
property bool shown:
621613
(chatView.bottommostVisibleIndex >= 0
622-
&& (scrollerArea.containsMouse || scrollAnimation.running))
623-
|| chatView.currentRequestedEvents > 0
614+
&& (scrollerArea.containsMouse || scrollAnimation.running))
615+
|| (room && room.requestedEventsCount > 0)
624616

625617
onShownChanged: {
626618
if (shown) {
@@ -648,10 +640,10 @@ Page {
648640
chatView.bottommostVisibleIndex))
649641
+ "\n" + qsTr("%Ln events cached", "", chatView.count)
650642
: "")
651-
+ (chatView.currentRequestedEvents > 0
643+
+ (room && room.requestedEventsCount > 0
652644
? (chatView.count > 0 ? "\n" : "")
653645
+ qsTr("%Ln events requested from the server",
654-
"", chatView.currentRequestedEvents)
646+
"", room.requestedEventsCount)
655647
: "")
656648
horizontalAlignment: Label.AlignRight
657649
}
@@ -720,7 +712,7 @@ Page {
720712
scrollFinisher.scrollViewTo(messageModel.readMarkerVisualIndex,
721713
ListView.Center)
722714
else
723-
room.getPreviousContent(chatView.count / 2) // FIXME, #799
715+
room.getHistory(chatView.count / 2) // FIXME, #799
724716
}
725717
}
726718
}

client/quaternionroom.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ QuaternionRoom::QuaternionRoom(Connection* connection, QString roomId,
2222
{
2323
connect(this, &Room::namesChanged,
2424
this, &QuaternionRoom::htmlSafeDisplayNameChanged);
25+
connect(this, &Room::eventsHistoryJobChanged,
26+
this, &QuaternionRoom::requestedEventsCountChanged);
2527
}
2628

2729
const QString& QuaternionRoom::cachedUserFilter() const
@@ -80,6 +82,17 @@ QString QuaternionRoom::htmlSafeDisplayName() const
8082
return displayName().toHtmlEscaped();
8183
}
8284

85+
int QuaternionRoom::requestedEventsCount() const
86+
{
87+
return eventsHistoryJob() != nullptr ? m_requestedEventsCount : 0;
88+
}
89+
90+
void QuaternionRoom::getHistory(int limit)
91+
{
92+
m_requestedEventsCount = limit;
93+
getPreviousContent(m_requestedEventsCount);
94+
}
95+
8396
void QuaternionRoom::onAddNewTimelineEvents(timeline_iter_t from)
8497
{
8598
std::for_each(from, messageEvents().cend(),

client/quaternionroom.h

+9
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ class QuaternionRoom: public Quotient::Room
1414
{
1515
Q_OBJECT
1616
Q_PROPERTY(QString htmlSafeDisplayName READ htmlSafeDisplayName NOTIFY htmlSafeDisplayNameChanged)
17+
Q_PROPERTY(int requestedEventsCount READ requestedEventsCount NOTIFY requestedEventsCountChanged)
1718
public:
1819
QuaternionRoom(Quotient::Connection* connection,
1920
QString roomId, Quotient::JoinState joinState);
@@ -29,16 +30,24 @@ class QuaternionRoom: public Quotient::Room
2930
bool force = false);
3031

3132
QString htmlSafeDisplayName() const;
33+
int requestedEventsCount() const;
34+
35+
public slots:
36+
// TODO, 0.0.96: move logic to libQuotient 0.9 and get rid of it here
37+
void getHistory(int limit);
3238

3339
signals:
3440
// Gotta wrap the Room::namesChanged signal because it has parameters
3541
// and moc cannot use signals with parameters defined in the parent
3642
// class as NOTIFY targets
3743
void htmlSafeDisplayNameChanged();
44+
// TODO, 0.0.96: same as for getHistory()
45+
void requestedEventsCountChanged();
3846

3947
private:
4048
QSet<const Quotient::RoomEvent*> highlights;
4149
QString m_cachedUserFilter;
50+
int m_requestedEventsCount = 0;
4251

4352
void onAddNewTimelineEvents(timeline_iter_t from) override;
4453
void onAddHistoricalTimelineEvents(rev_iter_t from) override;

0 commit comments

Comments
 (0)