Skip to content
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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

bun137
Copy link

@bun137 bun137 commented Nov 17, 2024

Adds the feature to show the hex code under the picker while picking the colour.

hyprpicker.mp4

hyprpicker with hexcode img1

hyprpicker with hexcode img2

@fufexan
Copy link
Member

fufexan commented Nov 17, 2024

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.

@danielwerg
Copy link

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.

@vaxerski
Copy link
Member

vaxerski commented Nov 17, 2024

2/10, should be right below the circle with a background. Right now it's barely readable and quite annoying

@bun137
Copy link
Author

bun137 commented Nov 17, 2024

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!

@bun137
Copy link
Author

bun137 commented Nov 17, 2024

2/10, should be right below the circle with a background. Right now it's barely readable and quite annoying

I'll make those changes.

@danielwerg
Copy link

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.

@vaxerski
Copy link
Member

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.

Yeah, enabled by default though, with a flag to disable

@fufexan
Copy link
Member

fufexan commented Nov 17, 2024

image
Looks really nice

@fufexan
Copy link
Member

fufexan commented Nov 17, 2024

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.

Right, this could have a check for the bottom-right corner, and move the text above and left of the cursor.

@bun137
Copy link
Author

bun137 commented Nov 17, 2024

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.

@danielwerg
Copy link

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);
+ }

@danielwerg
Copy link

Add some padding on left for the alpha box, otherwise # is touching the border.

@bun137
Copy link
Author

bun137 commented Nov 19, 2024

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.

@bun137
Copy link
Author

bun137 commented Nov 19, 2024

Here are some visuals!

1920x1080:

hyprpicker_fix.mp4

640x340:

hyprpicker_fix_640x340.mp4

3440x1440:

hyprpicker_fix_3440x1440.mp4

@danielwerg
Copy link

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.

@bun137
Copy link
Author

bun137 commented Nov 20, 2024

@vaxerski could you please review

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest ok

src/hyprpicker.cpp Outdated Show resolved Hide resolved
@danielwerg
Copy link

it still missing a flag to disable this feature, right?

@vaxerski
Copy link
Member

ah, right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants