For minimal shade selector, this patch adds indications of the first color, and the one that is now selected.
Details
Details
- Reviewers
dkazakov - Group Reviewers
Krita - Commits
- R37:c17ed2740971: Patch for the Bug 352869 shade selector indicator
Checking the color selectiom
Checking that when the colour is changed on the wheel, the sliders moves back to the initial position.
Checking the behaviour when the width of the docker is changed.
Checking the behaviour while the mouse goes out of the color selector
Diff Detail
Diff Detail
- Repository
- R37 Krita
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
You should add reviewers too :)
I won't be able to build this until monday, but krita:next tags all active commiters.
Comment Actions
Hi, Natalya!
I checked your patch and it looks really good! There is one blocker bug still. If you press a mouse button, move the mouse out of the widget and then release the button, the widget will end up in an inconsistent state. I added a comment about how to fix that. The other comments are just styling nit picking :)
We could also talk to Scott and Wolthera about the look of the controls. I guess they should either be XOR'ed over the background (if it is possible by Qt) or just their color should be adjusted according to some rule depending on the average HSV around them (granted we know the color of the background here).
Probably we could also make the shape of the two handles different somehow...
But these two comments are for the future work after the main patch is in master.
| plugins/extensions/dockers/advancedcolorselector/kis_shade_selector_line.cpp | ||
|---|---|---|
| 163 | if (qAbs(i) <= 5) { | |
| 209 | m_new.QSize::width() is really excessive here. QSize doesn't have complex operator overloads. | |
| 233 | You can use this line instead: if (e->buttons() & Qt::LeftButton) Using such booleans is dangerous because the user can press/release the mouse while being outsize the widget, so it can get into an inconsistent state. In ideal case, you should catch enterEvent() and leaveEvent() to catch these corner-cases. But in your specific case, testing for buttons() should be enough. | |