Details
- Reviewers
sander - Group Reviewers
Okular - Commits
- R223:1b89f220ef1b: Add icons for line annotation arrow styles to combo box
- open "Edit annotation tool" for straight line and check combo box items for start and stop style
- test on HiDPI
Diff Detail
- Repository
- R223 Okular
- Branch
- feature/lineannoticons
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 12261 Build 12279: arc lint + arc unit
ui/annotationwidgets.cpp | ||
---|---|---|
539 | Would it make sense to put all of this in a const variable outside of the loop? |
The UI looks good and I understand the code. :)
For dark color themes: maybe the color of the adjacent text would be good as foreground color for the icons. Don’t know, but maybe QGuiApplication::pallete().color(QPalette::WindowText)?
Tried your suggestion, looks good with breeze dark. Would you consider it important to connect to QGuiApplication::paletteChanged, to follow theme changes immediately?
Btw., after a local rebase Phabricator now shows ui/pagepainter.{h,cpp} as part of this D21416, but I actually touched these files only in the parent D21248. Don't know how to correct this.
Not wrong but not important. How often does one change the color theme while chosing a line ending? I could only imagine that the color theme changes automatically based on enviromnent light sensors, for people who work in a vehicle passing many tunnels. But how often does one chose the line ending while entering/leaving a tunnel the same time?
Yes:) Played around a bit, looks like many widgets in Okular don't react to a theme change immediately (only after shell restart). So implementing paletteChanged for arrow icons would make it worse, unless we fixed the surrounding widgets first.
Guess D21416 is ready now? If someone presses accept (also on D21248), I'll land it.
I'm afraid MSVC doesn't like this change - could someone please take a look?
https://build.kde.org/view/Failing/job/Applications/job/okular/job/kf5-qt5%20WindowsMSVCQt5.11/161/console
This is causing the Dependency Build for Extragear on Windows to fail, so a speedy resolution would be appreciated.
https://build.kde.org/view/Failing/job/Administration/job/Dependency%20Build%20Extragear%20kf5-qt5%20WindowsMSVCQt5.11/
Thanks for the quick update. Unfortunately I don't have a way of easily testing proposed patches, but in theory that should solve it yes.