Allow arbitrary HTML color, and setting default with picker #24
Conversation
annotations.py
Outdated
| # Currently, annotations only store the foreground color. If it's not a known color, then | ||
| # we should pick the background color as black or white based on the overall lightness of | ||
| # the color. | ||
| color = QColor(dt_bgcolor) | ||
| if color.lightness() > 128: | ||
| dt_fgcolor = "#000000" | ||
| else: | ||
| dt_fgcolor = "#FFFFFF" |
There was a problem hiding this comment.
Example of how this works:
As you can see, white gets a black foreground. The others get white.
This introduces a default bg and fg color setting, and also allows specifying an HTML color as well as the existing hard coded shades.
|
Actually, I like that. But I'm not 100% sure if we should allow selecting custom foreground color since we have a nice auto-detection code now. @stbenjam , you'd better also post a link to every PR you create to the https://www.mobileread.com/forums/showthread.php?t=241206 as well, because @davidfor isn't getting notifications from the GitHub. |
| try: | ||
| from PyQt5.QtGui import QColor | ||
| except ImportError: | ||
| from PyQt4.QtGui import QColor |
There was a problem hiding this comment.
Is this support for different Python versions used by Calibre now or in the future?
There was a problem hiding this comment.
Right, this is how all Qt dependencies are imported in this plugin, not just here.
|
|
||
| COLOR_MAP = { | ||
| 'Blue': {'bg': '#b1ccf3', 'fg': 'black'}, | ||
| 'Default': {'bg': 'transparent', 'fg': 'black'}, |
There was a problem hiding this comment.
Are you sure this removed entry isn't used anywhere else?
| if agroup.highlightcolor is None: | ||
| msg = "No highlight color specified, using Default" | ||
| else: | ||
| msg = "Unknown color '%s' specified" % agroup.highlightcolor |
There was a problem hiding this comment.
If the case with if agroup.highlightcolor is None: is still possible, then we should keep the original logging code (the one being highlighted here) instead of new msg = "Unknown color '%s' specified, using default" % agroup.highlightcolor code.
There was a problem hiding this comment.
Sure I added the specific None case back
The calculated foreground is only white or black, I think I'd rather give the user an option in case for whatever reason they want burgundy on yellow. |
|
@stbenjam , I completely understand you. Please be patient. We're not in a hurry here with @davidfor being very occupied with other projects and not being able to timely review any PR coming in. I'm glad, that you're able to implement a Moon+ Reader integration, that works for you. Please don't let it disappear and not make any other Moon+ Reader + Calibre user happy. It will be reviewed eventually I guess. |
2 participants
Extracted and improved my coloring code from #23.
This introduces a default bg and fg color setting exposed to the user. It
also allows specifying an HTML color as well as the existing hard coded
shades for any reader that wants to.
When an annotation highlight is an HTML color, it's lightness is calculated
and then either black or white is chosen to allow readability.
Video: https://www.youtube.com/watch?v=TVRstBtsMl8
I still need to update docs, but it requires new screenshots. I do not
have access to a mac, for consistency, would you like to take the
screen shot? Or are you OK with showing it from Linux?