[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kwidgetsaddons] src: Refactor and fix the feature introduced in bug 171343
From:       Weng Xuetian <wengxt () gmail ! com>
Date:       2015-12-04 16:26:03
Message-ID: E1a4tBL-0006dA-Ar () scm ! kde ! org
[Download RAW message or body]

Git commit efb14cb69ad53bcdb4f3f7b4f2439183d27ebb05 by Weng Xuetian.
Committed on 04/12/2015 at 16:19.
Pushed by xuetianweng into branch 'master'.

Refactor and fix the feature introduced in bug 171343

1. Enable this feature only if half step is enabled. Otherwise user can
only select full star, there is no point enable this feature.
2. Fix and merge the logic, especially for the part in mouseMoveEvent.
Previously mouseMoveEvent is calculating the new rating based on current
hover rating, which should use the current rating instead. And the logic
is hard for people to interpret without comment. Thus, this part of code
is refactored to be simpler and more readable with comment.

REVIEW: 126222
CCBUG: 171343

M  +2    -2    src/kratingpainter.cpp
M  +35   -32   src/kratingwidget.cpp

http://commits.kde.org/kwidgetsaddons/efb14cb69ad53bcdb4f3f7b4f2439183d27ebb05

diff --git a/src/kratingpainter.cpp b/src/kratingpainter.cpp
index 1a4378a..caa9585 100644
--- a/src/kratingpainter.cpp
+++ b/src/kratingpainter.cpp
@@ -212,7 +212,7 @@ void KRatingPainter::paint(QPainter *painter, const QRect &rect, \
int rating, int  
     int numUsedStars = d->bHalfSteps ? d->maxRating / 2 : d->maxRating;
 
-    if (hoverRating > 0 && hoverRating < rating) {
+    if (hoverRating >= 0 && hoverRating < rating) {
         int tmp = hoverRating;
         hoverRating = rating;
         rating = tmp;
@@ -242,7 +242,7 @@ void KRatingPainter::paint(QPainter *painter, const QRect &rect, \
int rating, int  
     int numHoverStars = 0;
     bool halfHover = false;
-    if (hoverRating > 0 && rating != hoverRating && d->isEnabled) {
+    if (hoverRating >= 0 && rating != hoverRating && d->isEnabled) {
         numHoverStars = d->bHalfSteps ? hoverRating / 2 : hoverRating;
         halfHover = d->bHalfSteps && hoverRating % 2;
 
diff --git a/src/kratingwidget.cpp b/src/kratingwidget.cpp
index d0d2903..d12a8f1 100644
--- a/src/kratingwidget.cpp
+++ b/src/kratingwidget.cpp
@@ -178,27 +178,41 @@ void KRatingWidget::setOnlyPaintFullSteps(bool fs)
 }
 #endif
 
+static inline int adjustedHoverRating(bool halfStep, int hoverRating, int rating)
+{
+    // intentionally skip zero, or half step is disabled.
+    if (!halfStep || hoverRating == 0) {
+        return hoverRating;
+    }
+
+    // See bug 171343, if we click on a star we want it to be full star, click again
+    // make it half, click third time make it clear.
+
+    // round up hoverRating to next full star rating
+    const int hoveredFullStarRating = hoverRating + (hoverRating % 2);
+    // Check if the star under mouse is the last half or whole star of the rating.
+    if (hoveredFullStarRating == rating || hoveredFullStarRating == rating + 1) {
+        // If current pointed star is not empty, next rating will be rating - 1
+        // if we point at 4th star and rating is 8 (4 star), next click will make it \
7 +        // if we point at 4th star and rating is 7 (3.5 star), next click will \
make it 6 +        hoverRating = rating - 1;
+    } else {
+        // otherwise make it a full star rating
+        hoverRating = hoveredFullStarRating;
+    }
+    return hoverRating;
+}
+
 void KRatingWidget::mousePressEvent(QMouseEvent *e)
 {
     if (e->button() == Qt::LeftButton) {
-        const int prevRating = d->rating;
-        d->hoverRating = d->ratingPainter.ratingFromPosition(contentsRect(), \
                e->pos());
-        if (!(d->hoverRating % 2)) {
-            if (d->hoverRating == prevRating + 1) {
-                setRating(d->hoverRating - 2);
-            } else if (d->hoverRating == prevRating) {
-                setRating(d->hoverRating - 1);
-            } else {
-                setRating(d->hoverRating);
-            }
-        } else {
-            if (d->hoverRating == prevRating - 1) {
-                setRating(d->hoverRating);
-            } else if (d->hoverRating == prevRating) {
-                setRating(d->hoverRating - 1);
-            } else {
-                setRating(d->hoverRating + 1);
-            }
+        d->hoverRating = adjustedHoverRating(halfStepsEnabled(),
+                                             \
d->ratingPainter.ratingFromPosition(contentsRect(), e->pos()), +                      \
d->rating); +        // avoid set a rating to something less than zero, it may happen \
if widget is scaled and +        // mouse is clicked outside the star region.
+        if (d->hoverRating >= 0) {
+            setRating(d->hoverRating);
         }
     }
 }
@@ -207,20 +221,9 @@ void KRatingWidget::mouseMoveEvent(QMouseEvent *e)
 {
     // when moving the mouse we show the user what the result of clicking will be
     const int prevHoverRating = d->hoverRating;
-    d->hoverRating = d->ratingPainter.ratingFromPosition(contentsRect(), e->pos());
-    if (!(d->hoverRating % 2)) {
-        if (d->hoverRating == prevHoverRating + 1) {
-            d->hoverRating -= 2;
-        } else if (d->hoverRating == prevHoverRating) {
-            d->hoverRating -= 1;
-        }
-    } else {
-        if (d->hoverRating == prevHoverRating) {
-            d->hoverRating -= 1;
-        } else {
-            d->hoverRating += 1;
-        }
-    }
+    d->hoverRating = adjustedHoverRating(halfStepsEnabled(),
+                                         \
d->ratingPainter.ratingFromPosition(contentsRect(), e->pos()), +                      \
d->rating);  if (d->hoverRating != prevHoverRating) {
         update();
     }


[prev in list] [next in list] [prev in thread] [next in thread]