From 8202cd0d9969fb88becb8aeaac607fc56697b541 Mon Sep 17 00:00:00 2001 From: nerix Date: Sun, 12 May 2024 12:52:58 +0200 Subject: [PATCH] refactor: cleanup and document `Scrollbar` (#5334) Co-authored-by: Rasmus Karlsson Co-authored-by: Daniel Sage --- CHANGELOG.md | 1 + src/widgets/Scrollbar.cpp | 325 ++++++++++------------- src/widgets/Scrollbar.hpp | 140 ++++++++-- src/widgets/dialogs/EmotePopup.cpp | 4 +- src/widgets/dialogs/ReplyThreadPopup.cpp | 4 +- src/widgets/dialogs/UserInfoPopup.cpp | 4 +- src/widgets/helper/ChannelView.cpp | 6 +- src/widgets/splits/Split.cpp | 4 +- tests/CMakeLists.txt | 1 + tests/src/ModerationAction.cpp | 2 +- tests/src/Scrollbar.cpp | 187 +++++++++++++ 11 files changed, 453 insertions(+), 225 deletions(-) create mode 100644 tests/src/Scrollbar.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index ea438bd61..631b3eb1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Bugfix: If a network request errors with 200 OK, Qt's error code is now reported instead of the HTTP status. (#5378) - Dev: Add doxygen build target. (#5377) - Dev: Make printing of strings in tests easier. (#5379) +- Dev: Refactor and document `Scrollbar`. (#5334) ## 2.5.1 diff --git a/src/widgets/Scrollbar.cpp b/src/widgets/Scrollbar.cpp index 6ca0e248e..e1492b673 100644 --- a/src/widgets/Scrollbar.cpp +++ b/src/widgets/Scrollbar.cpp @@ -4,7 +4,6 @@ #include "singletons/Settings.hpp" #include "singletons/Theme.hpp" #include "singletons/WindowManager.hpp" -#include "util/Clamp.hpp" #include "widgets/helper/ChannelView.hpp" #include @@ -13,7 +12,19 @@ #include -#define MIN_THUMB_HEIGHT 10 +namespace { + +constexpr int MIN_THUMB_HEIGHT = 10; + +/// Amount of messages to move by when clicking on the track +constexpr qreal SCROLL_DELTA = 5.0; + +bool areClose(auto a, auto b) +{ + return std::abs(a - b) <= 0.0001; +} + +} // namespace namespace chatterino { @@ -22,40 +33,51 @@ Scrollbar::Scrollbar(size_t messagesLimit, ChannelView *parent) , currentValueAnimation_(this, "currentValue_") , highlights_(messagesLimit) { - this->resize(int(16 * this->scale()), 100); + this->resize(static_cast(16 * this->scale()), 100); this->currentValueAnimation_.setDuration(150); this->currentValueAnimation_.setEasingCurve( QEasingCurve(QEasingCurve::OutCubic)); connect(&this->currentValueAnimation_, &QAbstractAnimation::finished, this, - &Scrollbar::resetMaximum); + &Scrollbar::resetBounds); this->setMouseTracking(true); } +boost::circular_buffer Scrollbar::getHighlights() const +{ + return this->highlights_; +} + void Scrollbar::addHighlight(ScrollbarHighlight highlight) { - this->highlights_.pushBack(highlight); + this->highlights_.push_back(std::move(highlight)); } void Scrollbar::addHighlightsAtStart( - const std::vector &_highlights) + const std::vector &highlights) { - this->highlights_.pushFront(_highlights); + size_t nItems = std::min(highlights.size(), this->highlights_.capacity() - + this->highlights_.size()); + + if (nItems == 0) + { + return; + } + + for (size_t i = 0; i < nItems; i++) + { + this->highlights_.push_front(highlights[highlights.size() - 1 - i]); + } } void Scrollbar::replaceHighlight(size_t index, ScrollbarHighlight replacement) { - this->highlights_.replaceItem(index, replacement); -} + if (this->highlights_.size() <= index) + { + return; + } -void Scrollbar::pauseHighlights() -{ - this->highlightsPaused_ = true; -} - -void Scrollbar::unpauseHighlights() -{ - this->highlightsPaused_ = false; + this->highlights_[index] = std::move(replacement); } void Scrollbar::clearHighlights() @@ -63,16 +85,6 @@ void Scrollbar::clearHighlights() this->highlights_.clear(); } -LimitedQueueSnapshot &Scrollbar::getHighlightSnapshot() -{ - if (!this->highlightsPaused_) - { - this->highlightSnapshot_ = this->highlights_.getSnapshot(); - } - - return this->highlightSnapshot_; -} - void Scrollbar::scrollToBottom(bool animate) { this->setDesiredValue(this->getBottom(), animate); @@ -102,7 +114,7 @@ void Scrollbar::offsetMaximum(qreal value) this->updateScroll(); } -void Scrollbar::resetMaximum() +void Scrollbar::resetBounds() { if (this->minimum_ > 0) { @@ -132,26 +144,19 @@ void Scrollbar::offsetMinimum(qreal value) this->updateScroll(); } -void Scrollbar::setLargeChange(qreal value) +void Scrollbar::setPageSize(qreal value) { - this->largeChange_ = value; - - this->updateScroll(); -} - -void Scrollbar::setSmallChange(qreal value) -{ - this->smallChange_ = value; + this->pageSize_ = value; this->updateScroll(); } void Scrollbar::setDesiredValue(qreal value, bool animated) { - value = std::max(this->minimum_, std::min(this->getBottom(), value)); - - if (std::abs(this->currentValue_ - value) <= 0.0001) + value = std::clamp(value, this->minimum_, this->getBottom()); + if (areClose(this->currentValue_, value)) { + // value has not changed return; } @@ -159,7 +164,7 @@ void Scrollbar::setDesiredValue(qreal value, bool animated) this->desiredValueChanged_.invoke(); - this->atBottom_ = (this->getBottom() - value) <= 0.0001; + this->atBottom_ = areClose(this->getBottom(), value); if (animated && getSettings()->enableSmoothScrolling) { @@ -178,7 +183,7 @@ void Scrollbar::setDesiredValue(qreal value, bool animated) else { this->setCurrentValue(value); - this->resetMaximum(); + this->resetBounds(); } } } @@ -193,19 +198,14 @@ qreal Scrollbar::getMinimum() const return this->minimum_; } -qreal Scrollbar::getLargeChange() const +qreal Scrollbar::getPageSize() const { - return this->largeChange_; + return this->pageSize_; } qreal Scrollbar::getBottom() const { - return this->maximum_ - this->largeChange_; -} - -qreal Scrollbar::getSmallChange() const -{ - return this->smallChange_; + return this->maximum_ - this->pageSize_; } qreal Scrollbar::getDesiredValue() const @@ -222,8 +222,8 @@ qreal Scrollbar::getRelativeCurrentValue() const { // currentValue - minimum can be negative if minimum is incremented while // scrolling up to or down from the top when smooth scrolling is enabled. - return clamp(this->currentValue_ - this->minimum_, qreal(0.0), - this->currentValue_); + return std::clamp(this->currentValue_ - this->minimum_, 0.0, + this->currentValue_); } void Scrollbar::offset(qreal value) @@ -244,9 +244,9 @@ pajlada::Signals::NoArgSignal &Scrollbar::getDesiredValueChanged() void Scrollbar::setCurrentValue(qreal value) { value = std::max(this->minimum_, std::min(this->getBottom(), value)); - - if (std::abs(this->currentValue_ - value) <= 0.0001) + if (areClose(this->currentValue_, value)) { + // value has not changed return; } @@ -258,21 +258,24 @@ void Scrollbar::setCurrentValue(qreal value) void Scrollbar::printCurrentState(const QString &prefix) const { - qCDebug(chatterinoWidget) - << prefix // - << "Current value: " << this->getCurrentValue() // - << ". Maximum: " << this->getMaximum() // - << ". Minimum: " << this->getMinimum() // - << ". Large change: " << this->getLargeChange(); // + qCDebug(chatterinoWidget).nospace().noquote() + << prefix // + << " { currentValue: " << this->getCurrentValue() // + << ", desiredValue: " << this->getDesiredValue() // + << ", maximum: " << this->getMaximum() // + << ", minimum: " << this->getMinimum() // + << ", pageSize: " << this->getPageSize() // + << " }"; } -void Scrollbar::paintEvent(QPaintEvent *) +void Scrollbar::paintEvent(QPaintEvent * /*event*/) { - bool mouseOver = this->mouseOverIndex_ != -1; - int xOffset = mouseOver ? 0 : width() - int(4 * this->scale()); + bool mouseOver = this->mouseOverLocation_ != MouseLocation::Outside; + int xOffset = + mouseOver ? 0 : this->width() - static_cast(4.0F * this->scale()); QPainter painter(this); - painter.fillRect(rect(), this->theme->scrollbars.background); + painter.fillRect(this->rect(), this->theme->scrollbars.background); bool enableRedeemedHighlights = getSettings()->enableRedeemedHighlight; bool enableFirstMessageHighlights = @@ -280,16 +283,10 @@ void Scrollbar::paintEvent(QPaintEvent *) bool enableElevatedMessageHighlights = getSettings()->enableElevatedMessageHighlight; - // painter.fillRect(QRect(xOffset, 0, width(), this->buttonHeight), - // this->themeManager->ScrollbarArrow); - // painter.fillRect(QRect(xOffset, height() - this->buttonHeight, - // width(), this->buttonHeight), - // this->themeManager->ScrollbarArrow); - this->thumbRect_.setX(xOffset); // mouse over thumb - if (this->mouseDownIndex_ == 2) + if (this->mouseDownLocation_ == MouseLocation::InsideThumb) { painter.fillRect(this->thumbRect_, this->theme->scrollbars.thumbSelected); @@ -301,23 +298,21 @@ void Scrollbar::paintEvent(QPaintEvent *) } // draw highlights - auto &snapshot = this->getHighlightSnapshot(); - size_t snapshotLength = snapshot.size(); - - if (snapshotLength == 0) + if (this->highlights_.empty()) { return; } + size_t nHighlights = this->highlights_.size(); int w = this->width(); - float y = 0; - float dY = float(this->height()) / float(snapshotLength); + float dY = + static_cast(this->height()) / static_cast(nHighlights); int highlightHeight = - int(std::ceil(std::max(this->scale() * 2, dY))); + static_cast(std::ceil(std::max(this->scale() * 2.0F, dY))); - for (size_t i = 0; i < snapshotLength; i++, y += dY) + for (size_t i = 0; i < nHighlights; i++) { - ScrollbarHighlight const &highlight = snapshot[i]; + const auto &highlight = this->highlights_[i]; if (highlight.isNull()) { @@ -344,16 +339,16 @@ void Scrollbar::paintEvent(QPaintEvent *) QColor color = highlight.getColor(); color.setAlpha(255); + int y = static_cast(dY * static_cast(i)); switch (highlight.getStyle()) { case ScrollbarHighlight::Default: { - painter.fillRect(w / 8 * 3, int(y), w / 4, highlightHeight, - color); + painter.fillRect(w / 8 * 3, y, w / 4, highlightHeight, color); } break; case ScrollbarHighlight::Line: { - painter.fillRect(0, int(y), w, 1, color); + painter.fillRect(0, y, w, 1, color); } break; @@ -362,52 +357,30 @@ void Scrollbar::paintEvent(QPaintEvent *) } } -void Scrollbar::resizeEvent(QResizeEvent *) +void Scrollbar::resizeEvent(QResizeEvent * /*event*/) { - this->resize(int(16 * this->scale()), this->height()); + this->resize(static_cast(16 * this->scale()), this->height()); } void Scrollbar::mouseMoveEvent(QMouseEvent *event) { - if (this->mouseDownIndex_ == -1) + if (this->mouseDownLocation_ == MouseLocation::Outside) { - int y = event->pos().y(); - - auto oldIndex = this->mouseOverIndex_; - - if (y < this->buttonHeight_) - { - this->mouseOverIndex_ = 0; - } - else if (y < this->thumbRect_.y()) - { - this->mouseOverIndex_ = 1; - } - else if (this->thumbRect_.contains(2, y)) - { - this->mouseOverIndex_ = 2; - } - else if (y < height() - this->buttonHeight_) - { - this->mouseOverIndex_ = 3; - } - else - { - this->mouseOverIndex_ = 4; - } - - if (oldIndex != this->mouseOverIndex_) + auto moveLocation = this->locationOfMouseEvent(event); + if (this->mouseOverLocation_ != moveLocation) { + this->mouseOverLocation_ = moveLocation; this->update(); } } - else if (this->mouseDownIndex_ == 2) + else if (this->mouseDownLocation_ == MouseLocation::InsideThumb) { - int delta = event->pos().y() - this->lastMousePosition_.y(); + qreal delta = + static_cast(event->pos().y() - this->lastMousePosition_.y()); this->setDesiredValue( this->desiredValue_ + - (qreal(delta) / std::max(0.00000002, this->trackHeight_)) * + (delta / std::max(0.00000002, this->trackHeight_)) * this->maximum_); } @@ -416,98 +389,80 @@ void Scrollbar::mouseMoveEvent(QMouseEvent *event) void Scrollbar::mousePressEvent(QMouseEvent *event) { - int y = event->pos().y(); - - if (y < this->buttonHeight_) - { - this->mouseDownIndex_ = 0; - } - else if (y < this->thumbRect_.y()) - { - this->mouseDownIndex_ = 1; - } - else if (this->thumbRect_.contains(2, y)) - { - this->mouseDownIndex_ = 2; - } - else if (y < height() - this->buttonHeight_) - { - this->mouseDownIndex_ = 3; - } - else - { - this->mouseDownIndex_ = 4; - } + this->mouseDownLocation_ = this->locationOfMouseEvent(event); + this->update(); } void Scrollbar::mouseReleaseEvent(QMouseEvent *event) { - int y = event->pos().y(); - - if (y < this->buttonHeight_) + auto releaseLocation = this->locationOfMouseEvent(event); + if (this->mouseDownLocation_ != releaseLocation) { - if (this->mouseDownIndex_ == 0) - { - this->setDesiredValue(this->desiredValue_ - this->smallChange_, - true); - } - } - else if (y < this->thumbRect_.y()) - { - if (this->mouseDownIndex_ == 1) - { - this->setDesiredValue(this->desiredValue_ - this->smallChange_, - true); - } - } - else if (this->thumbRect_.contains(2, y)) - { - // do nothing - } - else if (y < height() - this->buttonHeight_) - { - if (this->mouseDownIndex_ == 3) - { - this->setDesiredValue(this->desiredValue_ + this->smallChange_, - true); - } - } - else - { - if (this->mouseDownIndex_ == 4) - { - this->setDesiredValue(this->desiredValue_ + this->smallChange_, - true); - } + // Ignore event. User released the mouse from a different spot than + // they first clicked. For example, they clicked above the thumb, + // changed their mind, dragged the mouse below the thumb, and released. + this->mouseDownLocation_ = MouseLocation::Outside; + return; } - this->mouseDownIndex_ = -1; + switch (releaseLocation) + { + case MouseLocation::AboveThumb: + // Move scrollbar up a small bit. + this->setDesiredValue(this->desiredValue_ - SCROLL_DELTA, true); + break; + case MouseLocation::BelowThumb: + // Move scrollbar down a small bit. + this->setDesiredValue(this->desiredValue_ + SCROLL_DELTA, true); + break; + default: + break; + } + this->mouseDownLocation_ = MouseLocation::Outside; this->update(); } -void Scrollbar::leaveEvent(QEvent *) +void Scrollbar::leaveEvent(QEvent * /*event*/) { - this->mouseOverIndex_ = -1; - + this->mouseOverLocation_ = MouseLocation::Outside; this->update(); } void Scrollbar::updateScroll() { - this->trackHeight_ = this->height() - this->buttonHeight_ - - this->buttonHeight_ - MIN_THUMB_HEIGHT - 1; + this->trackHeight_ = this->height() - MIN_THUMB_HEIGHT - 1; auto div = std::max(0.0000001, this->maximum_ - this->minimum_); - this->thumbRect_ = QRect( - 0, - int((this->getRelativeCurrentValue()) / div * this->trackHeight_) + 1 + - this->buttonHeight_, - this->width(), - int(this->largeChange_ / div * this->trackHeight_) + MIN_THUMB_HEIGHT); + this->thumbRect_ = + QRect(0, + static_cast((this->getRelativeCurrentValue()) / div * + this->trackHeight_) + + 1, + this->width(), + static_cast(this->pageSize_ / div * this->trackHeight_) + + MIN_THUMB_HEIGHT); this->update(); } +Scrollbar::MouseLocation Scrollbar::locationOfMouseEvent( + QMouseEvent *event) const +{ + int y = event->pos().y(); + + if (y < this->thumbRect_.y()) + { + return MouseLocation::AboveThumb; + } + + if (this->thumbRect_.contains(2, y)) + { + return MouseLocation::InsideThumb; + } + + return MouseLocation::BelowThumb; +} + } // namespace chatterino diff --git a/src/widgets/Scrollbar.hpp b/src/widgets/Scrollbar.hpp index d025539c1..08a843586 100644 --- a/src/widgets/Scrollbar.hpp +++ b/src/widgets/Scrollbar.hpp @@ -1,11 +1,10 @@ #pragma once -#include "messages/LimitedQueue.hpp" #include "widgets/BaseWidget.hpp" #include "widgets/helper/ScrollbarHighlight.hpp" +#include #include -#include #include #include @@ -13,41 +12,119 @@ namespace chatterino { class ChannelView; +/// @brief A scrollbar for views with partially laid out items +/// +/// This scrollbar is made for views that only lay out visible items. This is +/// the case for a @a ChannelView for example. There, only the visible messages +/// are laid out. For a traditional scrollbar, all messages would need to be +/// laid out to be able to compute the total height of all items. However, for +/// these messages this isn't possible. +/// +/// To avoid having to lay out all items, this scrollbar tracks the position of +/// the content in messages (as opposed to pixels). The position is given by +/// `currentValue` which refers to the index of the message at the top plus a +/// fraction inside the message. The position can be animated to have a smooth +/// scrolling effect. In this case, `currentValue` refers to the displayed +/// position and `desiredValue` refers to the position the scrollbar is set to +/// be at after the animation. The latter is used for example to check if the +/// scrollbar is at the bottom. +/// +/// `minimum` and `maximum` are used to map scrollbar positions to +/// (message-)buffer indices. The buffer is of size `maximum - minimum` and an +/// index is computed by `scrollbarPos - minimum` - thus a scrollbar position +/// of a message is at `index + minimum. +/// +/// @cond src-only +/// +/// The following illustrates a scrollbar in a channel view with seven +/// messages. The scrollbar is at the bottom. No animation is active, thus +/// `currentValue = desiredValue`. +/// +/// ┌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┐←╌╌╌ minimum +/// Alice: This message is quite = 0 +/// ┬ ╭─────────────────────────────────╮←╮ +/// │ │ long, so it gets wrapped │ ┆ +/// │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ ╰╌╌╌ currentValue +/// │ │ Bob: are you sure? │ = 0.5 +/// │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ = desiredValue +/// pageSize ╌╌╌┤ │ Alice: Works for me... try for │ = maximum +/// = 6.5 │ │ yourself │ - pageSize +/// │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ = bottom +/// │ │ Bob: I'm trying to get my really│ ⇒ atBottom = true +/// │ │ long message to wrap so I can │ +/// │ │ debug this issue I'm facing... │ +/// │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ +/// │ │ Bob: Omg it worked │ +/// │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤ +/// │ │ Alice: That's amazing! ╭┤ ┬ +/// │ │╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌││ ├╌╌ thumbRect.height() +/// │ │ Bob: you're right ╰┤ ┴ +/// ┴╭→╰─────────────────────────────────╯ +/// ┆ +/// maximum +/// = 7 +/// @endcond +/// +/// When messages are added at the bottom, both maximum and minimum are offset +/// by 1 and after a layout, the desired value is updated, causing the content +/// to move. Afterwards, the bounds are reset (potentially waiting for the +/// animation to finish). +/// +/// While scrolling is paused, the desired (and current) value won't be +/// updated. However, messages can still come in and "shift" the values in the +/// backing ring-buffer. If the current value would be used, the messages would +/// still shift upwards (just at a different offset). To avoid this, there's a +/// _relative current value_, which is `currentValue - minimum`. It's the +/// actual index of the top message in the buffer. Since the minimum is shifted +/// by 1 when messages come in, the view will remain idle (visually). class Scrollbar : public BaseWidget { Q_OBJECT public: - Scrollbar(size_t messagesLimit, ChannelView *parent = nullptr); + Scrollbar(size_t messagesLimit, ChannelView *parent); + /// Return a copy of the highlights + /// + /// Should only be used for tests + boost::circular_buffer getHighlights() const; void addHighlight(ScrollbarHighlight highlight); void addHighlightsAtStart( const std::vector &highlights_); void replaceHighlight(size_t index, ScrollbarHighlight replacement); - void pauseHighlights(); - void unpauseHighlights(); void clearHighlights(); void scrollToBottom(bool animate = false); void scrollToTop(bool animate = false); bool isAtBottom() const; + qreal getMaximum() const; void setMaximum(qreal value); void offsetMaximum(qreal value); - void resetMaximum(); + + qreal getMinimum() const; void setMinimum(qreal value); void offsetMinimum(qreal value); - void setLargeChange(qreal value); - void setSmallChange(qreal value); - void setDesiredValue(qreal value, bool animated = false); - qreal getMaximum() const; - qreal getMinimum() const; - qreal getLargeChange() const; - qreal getBottom() const; - qreal getSmallChange() const; + + void resetBounds(); + + qreal getPageSize() const; + void setPageSize(qreal value); + qreal getDesiredValue() const; + void setDesiredValue(qreal value, bool animated = false); + + /// The bottom-most scroll position + qreal getBottom() const; qreal getCurrentValue() const; + + /// @brief The current value relative to the minimum + /// + /// > currentValue - minimum + /// + /// This should be used as an index into a buffer of messages, as it is + /// unaffected by simultaneous shifts of minimum and maximum. qreal getRelativeCurrentValue() const; // offset the desired value without breaking smooth scolling @@ -56,47 +133,54 @@ public: pajlada::Signals::NoArgSignal &getDesiredValueChanged(); void setCurrentValue(qreal value); - void printCurrentState(const QString &prefix = QString()) const; + void printCurrentState( + const QString &prefix = QStringLiteral("Scrollbar")) const; Q_PROPERTY(qreal desiredValue_ READ getDesiredValue WRITE setDesiredValue) protected: - void paintEvent(QPaintEvent *) override; - void resizeEvent(QResizeEvent *) override; + void paintEvent(QPaintEvent *event) override; + void resizeEvent(QResizeEvent *event) override; void mouseMoveEvent(QMouseEvent *event) override; void mousePressEvent(QMouseEvent *event) override; void mouseReleaseEvent(QMouseEvent *event) override; - void leaveEvent(QEvent *) override; + void leaveEvent(QEvent *event) override; private: Q_PROPERTY(qreal currentValue_ READ getCurrentValue WRITE setCurrentValue) - LimitedQueueSnapshot &getHighlightSnapshot(); void updateScroll(); - QMutex mutex_; + enum class MouseLocation { + /// The mouse is positioned outside the scrollbar + Outside, + /// The mouse is positioned inside the scrollbar, but above the thumb (the thing you can drag inside the scrollbar) + AboveThumb, + /// The mouse is positioned inside the scrollbar, and on top of the thumb + InsideThumb, + /// The mouse is positioned inside the scrollbar, but below the thumb + BelowThumb, + }; + + MouseLocation locationOfMouseEvent(QMouseEvent *event) const; QPropertyAnimation currentValueAnimation_; - LimitedQueue highlights_; - bool highlightsPaused_{false}; - LimitedQueueSnapshot highlightSnapshot_; + boost::circular_buffer highlights_; bool atBottom_{false}; - int mouseOverIndex_ = -1; - int mouseDownIndex_ = -1; + MouseLocation mouseOverLocation_ = MouseLocation::Outside; + MouseLocation mouseDownLocation_ = MouseLocation::Outside; QPoint lastMousePosition_; - int buttonHeight_ = 0; int trackHeight_ = 100; QRect thumbRect_; qreal maximum_ = 0; qreal minimum_ = 0; - qreal largeChange_ = 0; - qreal smallChange_ = 5; + qreal pageSize_ = 0; qreal desiredValue_ = 0; qreal currentValue_ = 0; diff --git a/src/widgets/dialogs/EmotePopup.cpp b/src/widgets/dialogs/EmotePopup.cpp index 71a18a0a3..fd80d7e95 100644 --- a/src/widgets/dialogs/EmotePopup.cpp +++ b/src/widgets/dialogs/EmotePopup.cpp @@ -350,11 +350,11 @@ void EmotePopup::addShortcuts() auto &scrollbar = channelView->getScrollBar(); if (direction == "up") { - scrollbar.offset(-scrollbar.getLargeChange()); + scrollbar.offset(-scrollbar.getPageSize()); } else if (direction == "down") { - scrollbar.offset(scrollbar.getLargeChange()); + scrollbar.offset(scrollbar.getPageSize()); } else { diff --git a/src/widgets/dialogs/ReplyThreadPopup.cpp b/src/widgets/dialogs/ReplyThreadPopup.cpp index 32ee5a529..4d3dd3a83 100644 --- a/src/widgets/dialogs/ReplyThreadPopup.cpp +++ b/src/widgets/dialogs/ReplyThreadPopup.cpp @@ -51,11 +51,11 @@ ReplyThreadPopup::ReplyThreadPopup(bool closeAutomatically, Split *split) auto &scrollbar = this->ui_.threadView->getScrollBar(); if (direction == "up") { - scrollbar.offset(-scrollbar.getLargeChange()); + scrollbar.offset(-scrollbar.getPageSize()); } else if (direction == "down") { - scrollbar.offset(scrollbar.getLargeChange()); + scrollbar.offset(scrollbar.getPageSize()); } else { diff --git a/src/widgets/dialogs/UserInfoPopup.cpp b/src/widgets/dialogs/UserInfoPopup.cpp index 953be229c..fff4fefc1 100644 --- a/src/widgets/dialogs/UserInfoPopup.cpp +++ b/src/widgets/dialogs/UserInfoPopup.cpp @@ -164,11 +164,11 @@ UserInfoPopup::UserInfoPopup(bool closeAutomatically, Split *split) auto &scrollbar = this->ui_.latestMessages->getScrollBar(); if (direction == "up") { - scrollbar.offset(-scrollbar.getLargeChange()); + scrollbar.offset(-scrollbar.getPageSize()); } else if (direction == "down") { - scrollbar.offset(scrollbar.getLargeChange()); + scrollbar.offset(scrollbar.getPageSize()); } else { diff --git a/src/widgets/helper/ChannelView.cpp b/src/widgets/helper/ChannelView.cpp index c5a679727..819b7d0c0 100644 --- a/src/widgets/helper/ChannelView.cpp +++ b/src/widgets/helper/ChannelView.cpp @@ -744,7 +744,7 @@ void ChannelView::updateScrollbar( if (h < 0) // break condition { - this->scrollBar_->setLargeChange( + this->scrollBar_->setPageSize( (messages.size() - i) + qreal(h) / std::max(1, message->getHeight())); @@ -778,7 +778,7 @@ void ChannelView::clearMessages() // Clear all stored messages in this chat widget this->messages_.clear(); this->scrollBar_->clearHighlights(); - this->scrollBar_->resetMaximum(); + this->scrollBar_->resetBounds(); this->scrollBar_->setMaximum(0); this->scrollBar_->setMinimum(0); this->queueLayout(); @@ -1277,7 +1277,7 @@ void ChannelView::messagesUpdated() this->messages_.clear(); this->scrollBar_->clearHighlights(); - this->scrollBar_->resetMaximum(); + this->scrollBar_->resetBounds(); this->scrollBar_->setMaximum(qreal(snapshot.size())); this->scrollBar_->setMinimum(0); this->lastMessageHasAlternateBackground_ = false; diff --git a/src/widgets/splits/Split.cpp b/src/widgets/splits/Split.cpp index 81b0e2f65..1e34aefbf 100644 --- a/src/widgets/splits/Split.cpp +++ b/src/widgets/splits/Split.cpp @@ -553,11 +553,11 @@ void Split::addShortcuts() auto &scrollbar = this->getChannelView().getScrollBar(); if (direction == "up") { - scrollbar.offset(-scrollbar.getLargeChange()); + scrollbar.offset(-scrollbar.getPageSize()); } else if (direction == "down") { - scrollbar.offset(scrollbar.getLargeChange()); + scrollbar.offset(scrollbar.getPageSize()); } else { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8ea086b13..8288664df 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -43,6 +43,7 @@ set(test_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/MessageLayout.cpp ${CMAKE_CURRENT_LIST_DIR}/src/QMagicEnum.cpp ${CMAKE_CURRENT_LIST_DIR}/src/ModerationAction.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/Scrollbar.cpp # Add your new file above this line! ) diff --git a/tests/src/ModerationAction.cpp b/tests/src/ModerationAction.cpp index ce32fb39d..75daf8e3e 100644 --- a/tests/src/ModerationAction.cpp +++ b/tests/src/ModerationAction.cpp @@ -5,8 +5,8 @@ #include "singletons/Emotes.hpp" #include "singletons/Resources.hpp" #include "singletons/Settings.hpp" +#include "Test.hpp" -#include #include using namespace chatterino; diff --git a/tests/src/Scrollbar.cpp b/tests/src/Scrollbar.cpp new file mode 100644 index 000000000..98ca9a640 --- /dev/null +++ b/tests/src/Scrollbar.cpp @@ -0,0 +1,187 @@ +#include "widgets/Scrollbar.hpp" + +#include "Application.hpp" +#include "mocks/EmptyApplication.hpp" +#include "singletons/Fonts.hpp" +#include "singletons/Settings.hpp" +#include "singletons/Theme.hpp" +#include "singletons/WindowManager.hpp" +#include "Test.hpp" +#include "widgets/helper/ScrollbarHighlight.hpp" + +#include + +#include + +using namespace chatterino; + +namespace { + +class MockApplication : mock::EmptyApplication +{ +public: + MockApplication() + : settings(this->settingsDir.filePath("settings.json")) + , fonts(this->settings) + , windowManager(this->paths_) + { + } + Theme *getThemes() override + { + return &this->theme; + } + + Fonts *getFonts() override + { + return &this->fonts; + } + + WindowManager *getWindows() override + { + return &this->windowManager; + } + + Settings settings; + Theme theme; + Fonts fonts; + WindowManager windowManager; +}; + +} // namespace + +TEST(Scrollbar, AddHighlight) +{ + MockApplication mockApplication; + + Scrollbar scrollbar(10, nullptr); + EXPECT_EQ(scrollbar.getHighlights().size(), 0); + + for (int i = 0; i < 15; ++i) + { + auto color = std::make_shared(i, 0, 0); + ScrollbarHighlight scrollbarHighlight{color}; + scrollbar.addHighlight(scrollbarHighlight); + } + + EXPECT_EQ(scrollbar.getHighlights().size(), 10); + auto highlights = scrollbar.getHighlights(); + for (int i = 0; i < 10; ++i) + { + auto highlight = highlights[i]; + EXPECT_EQ(highlight.getColor().red(), i + 5); + } +} + +TEST(Scrollbar, AddHighlightsAtStart) +{ + MockApplication mockApplication; + + Scrollbar scrollbar(10, nullptr); + EXPECT_EQ(scrollbar.getHighlights().size(), 0); + + { + scrollbar.addHighlightsAtStart({ + { + std::make_shared(1, 0, 0), + }, + }); + auto highlights = scrollbar.getHighlights(); + EXPECT_EQ(highlights.size(), 1); + EXPECT_EQ(highlights[0].getColor().red(), 1); + } + + { + scrollbar.addHighlightsAtStart({ + { + std::make_shared(2, 0, 0), + }, + }); + auto highlights = scrollbar.getHighlights(); + EXPECT_EQ(highlights.size(), 2); + EXPECT_EQ(highlights[0].getColor().red(), 2); + EXPECT_EQ(highlights[1].getColor().red(), 1); + } + + { + scrollbar.addHighlightsAtStart({ + { + std::make_shared(4, 0, 0), + }, + { + std::make_shared(3, 0, 0), + }, + }); + auto highlights = scrollbar.getHighlights(); + EXPECT_EQ(highlights.size(), 4); + EXPECT_EQ(highlights[0].getColor().red(), 4); + EXPECT_EQ(highlights[1].getColor().red(), 3); + EXPECT_EQ(highlights[2].getColor().red(), 2); + EXPECT_EQ(highlights[3].getColor().red(), 1); + } + + { + // Adds as many as it can, in reverse order + scrollbar.addHighlightsAtStart({ + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(10, 0, 0)}, + {std::make_shared(9, 0, 0)}, + {std::make_shared(8, 0, 0)}, + {std::make_shared(7, 0, 0)}, + {std::make_shared(6, 0, 0)}, + {std::make_shared(5, 0, 0)}, + }); + auto highlights = scrollbar.getHighlights(); + EXPECT_EQ(highlights.size(), 10); + for (const auto &highlight : highlights) + { + std::cout << highlight.getColor().red() << '\n'; + } + EXPECT_EQ(highlights[0].getColor().red(), 10); + EXPECT_EQ(highlights[1].getColor().red(), 9); + EXPECT_EQ(highlights[2].getColor().red(), 8); + EXPECT_EQ(highlights[3].getColor().red(), 7); + EXPECT_EQ(highlights[4].getColor().red(), 6); + EXPECT_EQ(highlights[5].getColor().red(), 5); + EXPECT_EQ(highlights[6].getColor().red(), 4); + EXPECT_EQ(highlights[7].getColor().red(), 3); + EXPECT_EQ(highlights[8].getColor().red(), 2); + EXPECT_EQ(highlights[9].getColor().red(), 1); + } + + { + // Adds as many as it can, in reverse order + // Since the highlights are already full, nothing will be added + scrollbar.addHighlightsAtStart({ + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + {std::make_shared(255, 0, 0)}, + }); + auto highlights = scrollbar.getHighlights(); + EXPECT_EQ(highlights.size(), 10); + for (const auto &highlight : highlights) + { + std::cout << highlight.getColor().red() << '\n'; + } + EXPECT_EQ(highlights[0].getColor().red(), 10); + EXPECT_EQ(highlights[1].getColor().red(), 9); + EXPECT_EQ(highlights[2].getColor().red(), 8); + EXPECT_EQ(highlights[3].getColor().red(), 7); + EXPECT_EQ(highlights[4].getColor().red(), 6); + EXPECT_EQ(highlights[5].getColor().red(), 5); + EXPECT_EQ(highlights[6].getColor().red(), 4); + EXPECT_EQ(highlights[7].getColor().red(), 3); + EXPECT_EQ(highlights[8].getColor().red(), 2); + EXPECT_EQ(highlights[9].getColor().red(), 1); + } +}