Skip to content
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

Fix data flow #43

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 37 additions & 34 deletions src/widget/wstarrating.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@

WStarRating::WStarRating(const QString& group, QWidget* pParent)
: WWidget(pParent),
m_starRating(0, 5),
m_focused(false),
m_currRating(0) {
m_starCount(0),
m_visualStarRating(m_starCount, 5) {
// Controls to change the star rating with controllers.
// Note that 'group' maybe NULLPTR, e.g. when called from DlgTrackInfo,
// so only create rate change COs if there's a group passed when creating deck widgets.
Expand All @@ -38,22 +37,26 @@ QSize WStarRating::sizeHint() const {
QStyleOption option;
option.initFrom(this);
QSize widgetSize = style()->sizeFromContents(QStyle::CT_PushButton, &option,
m_starRating.sizeHint(), this);
m_visualStarRating.sizeHint(), this);

m_contentRect.setRect(
(widgetSize.width() - m_starRating.sizeHint().width()) / 2,
(widgetSize.height() - m_starRating.sizeHint().height()) / 2,
m_starRating.sizeHint().width(),
m_starRating.sizeHint().height()
(widgetSize.width() - m_visualStarRating.sizeHint().width()) / 2,
(widgetSize.height() - m_visualStarRating.sizeHint().height()) / 2,
m_visualStarRating.sizeHint().width(),
m_visualStarRating.sizeHint().height()
);

return widgetSize;
}

void WStarRating::slotSetRating(int rating) {
m_starRating.setStarCount(rating);
m_currRating = rating;
update();
void WStarRating::slotSetRating(int starCount) {
if (starCount == m_starCount) {
// Unchanged
return;
}
m_starCount = starCount;
resetVisualRating();
emit ratingChanged(starCount);
Comment on lines +57 to +59
Copy link
Owner

@ronso0 ronso0 May 19, 2023

Choose a reason for hiding this comment

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

shouldn't this be updateVisualrating()?
Also, emitting the signal again with the value we may just received doesn't make sense to me. What for?
It seems emit is required here only because you use slotSetRating instead of emit in mouseReleaseEvent.

}

void WStarRating::paintEvent(QPaintEvent * /*unused*/) {
Expand All @@ -64,70 +67,70 @@ void WStarRating::paintEvent(QPaintEvent * /*unused*/) {
painter.setBrush(option.palette.text());
painter.drawPrimitive(QStyle::PE_Widget, option);

m_starRating.paint(&painter, m_contentRect);
m_visualStarRating.paint(&painter, m_contentRect);
}

void WStarRating::mouseMoveEvent(QMouseEvent *event) {
m_focused = true;
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
int star = starAtPosition(event->position().toPoint().x());
#else
int star = starAtPosition(event->x());
#endif

if (star != m_starRating.starCount() && star != -1) {
m_starRating.setStarCount(star);
update();
if (star != -1) {
updateVisualRating(star);
}
}

void WStarRating::slotStarsUp(double v) {
if (v > 0 && m_currRating < m_starRating.maxStarCount()) {
int star = m_currRating + 1;
emit ratingChanged(star);
if (v > 0 && m_starCount < m_visualStarRating.maxStarCount()) {
slotSetRating(m_starCount + 1);
}
}

void WStarRating::slotStarsDown(double v) {
if (v > 0 && m_currRating > 0) {
int star = m_currRating - 1;
emit ratingChanged(star);
if (v > 0 && m_starCount > StarRating::kMinStarCount) {
slotSetRating(m_starCount - 1);
}
}

void WStarRating::leaveEvent(QEvent* /*unused*/) {
m_focused = false;
// reset to applied track rating
m_starRating.setStarCount(m_currRating);
resetVisualRating();
}

void WStarRating::updateVisualRating(int starCount) {
if (starCount == m_visualStarRating.starCount()) {
return;
}
m_visualStarRating.setStarCount(starCount);
update();
}

// The method uses basic linear algebra to find out which star is under the cursor.
int WStarRating::starAtPosition(int x) {
int WStarRating::starAtPosition(int x) const {
// If the mouse is very close to the left edge, set 0 stars.
if (x < m_starRating.sizeHint().width() * 0.05) {
if (x < m_visualStarRating.sizeHint().width() * 0.05) {
return 0;
}
int star = (x / (m_starRating.sizeHint().width() / m_starRating.maxStarCount())) + 1;
int star = (x / (m_visualStarRating.sizeHint().width() / m_visualStarRating.maxStarCount())) + 1;

if (star <= 0 || star > m_starRating.maxStarCount()) {
if (star <= 0 || star > m_visualStarRating.maxStarCount()) {
return 0;
}

return star;
}

void WStarRating::mouseReleaseEvent(QMouseEvent* /*unused*/) {
emit ratingChanged(m_starRating.starCount());
slotSetRating(m_visualStarRating.starCount());
Copy link
Owner

Choose a reason for hiding this comment

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

This feels wrong: we now store the stars without confirmation, as if it was an update from the owner/mediator. IMO it was correct before: emit the change (don't store it) and wait for the owner to confirm/reject/reset.

Copy link
Owner

Choose a reason for hiding this comment

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

Result of this implementation is that you can now set stars in the rating widget of an empty deck.

}

void WStarRating::fillDebugTooltip(QStringList* debug) {
WWidget::fillDebugTooltip(debug);

QString currentRating;
currentRating.setNum(m_currRating);
QString maximumRating = QString::number(m_starRating.maxStarCount());

currentRating.setNum(m_starCount);
QString maximumRating = QString::number(m_visualStarRating.maxStarCount());

*debug << QString("Rating: %1/%2").arg(currentRating, maximumRating);
}
18 changes: 11 additions & 7 deletions src/widget/wstarrating.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ class WStarRating : public WWidget {
QSize sizeHint() const override;

public slots:
void slotSetRating(int rating);
void slotSetRating(int starCount);

signals:
void ratingChanged(int rating);
void ratingChanged(int starCount);

private slots:
void slotStarsUp(double v);
Expand All @@ -40,13 +40,17 @@ class WStarRating : public WWidget {
void fillDebugTooltip(QStringList* debug) override;

private:
StarRating m_starRating;
bool m_focused;
int m_starCount;

StarRating m_visualStarRating;
mutable QRect m_contentRect;

int starAtPosition(int x);
int starAtPosition(int x) const;
void updateVisualRating(int starCount);
void resetVisualRating() {
updateVisualRating(m_starCount);
}

std::unique_ptr<ControlPushButton> m_pStarsUp;
std::unique_ptr<ControlPushButton> m_pStarsDown;

int m_currRating;
};