-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat: adds hexcode below the cursor #95
base: main
Are you sure you want to change the base?
Conversation
Looks nice! One thing I feel is missing is a background for the text. When you hover over white the text is not visible. A background with 0.5 alpha would make it more visible. |
Nice! What @fufexan said and is there a reason you put hex code inside the ring? I don't think I have a preference either way but was just wondering. |
2/10, should be right below the circle with a background. Right now it's barely readable and quite annoying |
Thank you! I've added background to the hex (made a simple rectangle behind the text). I was messing around tryna figure out the best place to put the hex code. Putting it at the bottom of the screen wasn't so great. Plus I had read a comment saying some folks use it to compare colours so kinda made sense to put in the ring! |
I'll make those changes. |
Oh great, no issues functionality-wise anymore for me. For design I would suggest trying to round corners a little, right now it looks a bit bold. I think this feature should be put behind a flag, I would want to use it 100% of the time but I don't think that going to be the case for most people. Good point on the hex code position! The only time user won't be able to see is when cursor is at top right corner of the screen or anywhere on X axis at the bottom. |
Yeah, enabled by default though, with a flag to disable |
Right, this could have a check for the bottom-right corner, and move the text above and left of the cursor. |
Thanks! Added the rounded corners to the rectangle, I'm working on having a check for the bottom so that I can move the text above the cursor. |
I don't know C++ but I did some hard-coding as a PoW - cairo_rectangle(PCAIRO, CLICKPOS.x, CLICKPOS.y + 20, 80, 30);
+ if (CLICKPOS.y > (PBUFFER->pixelSize.y - 50) && CLICKPOS.x >(PBUFFER->pixelSize.x - 100)) {
+ cairo_rectangle(PCAIRO, CLICKPOS.x - 80, CLICKPOS.y - 40, 80, 30);
+ } else if (CLICKPOS.y > (PBUFFER->pixelSize.y - 50)) {
+ cairo_rectangle(PCAIRO, CLICKPOS.x, CLICKPOS.y - 40, 80, 30);
+ } else if (CLICKPOS.x > (PBUFFER->pixelSize.x - 100)) {
+ cairo_rectangle(PCAIRO, CLICKPOS.x - 80, CLICKPOS.y + 20, 80, 30);
+ } else {
+ cairo_rectangle(PCAIRO, CLICKPOS.x, CLICKPOS.y + 20, 80, 30);
+ }
- cairo_move_to(PCAIRO, CLICKPOS.x, CLICKPOS.y + 40);
+ if (CLICKPOS.y > (PBUFFER->pixelSize.y - 50) && CLICKPOS.x > (PBUFFER->pixelSize.x - 100)) {
+ cairo_move_to(PCAIRO, CLICKPOS.x - 80, CLICKPOS.y - 20);
+ } else if (CLICKPOS.y > (PBUFFER->pixelSize.y - 50)) {
+ cairo_move_to(PCAIRO, CLICKPOS.x, CLICKPOS.y - 20);
+ } else if (CLICKPOS.x > (PBUFFER->pixelSize.x - 100)) {
+ cairo_rectangle(PCAIRO, CLICKPOS.x - 80, CLICKPOS.y + 40, 80, 30);
+ } else {
+ cairo_move_to(PCAIRO, CLICKPOS.x, CLICKPOS.y + 40);
+ } |
Add some padding on left for the alpha box, otherwise |
I added padding and tested the picker across various resolutions to ensure the hex code is visible everywhere. Also, I updated the hex code to render in uppercase. |
Here are some visuals!1920x1080: hyprpicker_fix.mp4640x340: hyprpicker_fix_640x340.mp43440x1440: hyprpicker_fix_3440x1440.mp4 |
Yeah, uppercase hex is better for consistency since that the only format hyprpicker can output right now #55. Looks perfect to me, need vaxry to review now. |
@vaxerski could you please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest ok
it still missing a flag to disable this feature, right? |
ah, right |
Adds the feature to show the hex code under the picker while picking the colour.
hyprpicker.mp4