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

continuation of #468 : checkbox radio button appearance update #681

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

awschult002
Copy link
Contributor

All credit goes to those referenced in other PRs (#468) . I have updated the original branch to remove conflicts with master. I have also verified the checkbox appearance with all SDL, GLFW, X11, and some GDI. All of the different styles seem to handle the update nicely.

Note: this might not be the best way to update things; but i couldn't push to the original and I figured that a 2 year old branch might not get much attention from the original author even if I did ask for a PR into that branch.

Please let me know if there is anything that I can do, as I really want to see this specific update get integrated

@RobLoach
Copy link
Contributor

RobLoach commented Sep 5, 2024

Thanks for revamping this. I like the direction, and it looks great on the OpenGL demos, but on some of the other renderers, it looks a bit off, and I'm not entirely sure why it's happening...

X11

Before

x11-before

After

x11-after

RawFB SDL

Before

rawfb-sdl-before

After

rawfb-sdl-after

@awschult002
Copy link
Contributor Author

i took a look into the X11 example and I cant find anything in specific that would be causing this. It could be something particular with how the X11 example is handling the draw call. However, your SDL example is interesting as both seem to have very similar problems.

I will take another look to see if i can't find anything. However, i must ask. is this a show stopper? would the slight variation in the render out weight the benefit of the overall change? My personal take is, no and that clarity of the selection is more important.

The new checkbox toggle look was off by a couple of pixels. This commit fixes that by removing the "line thickness" operation performed on its width before drawing.
The new checkbox toggle look was off by a couple of pixels. This commit fixes that by removing the "line thickness" operation performed on its width before drawing.
@awschult002
Copy link
Contributor Author

Thanks for revamping this. I like the direction, and it looks great on the OpenGL demos, but on some of the other renderers, it looks a bit off, and I'm not entirely sure why it's happening...

X11

Before

x11-before

After

x11-after

RawFB SDL

Before

rawfb-sdl-before

After

rawfb-sdl-after

I figured out the issue with the X11 demos and have provided a fix in this PR. However, There seems to be some different kind of issue with RAW_FB stuff and that I haven't been able to solve yet. This doesn't seem to be a problem with the toggle code, more of a draw issue specific to RAW_FB.

BEFORE

image

After

image

@RobLoach
Copy link
Contributor

Great find. Thanks for the updates. Do you think this is ready enough to merge in?

@awschult002
Copy link
Contributor Author

Definitely good to go

@RobLoach
Copy link
Contributor

RobLoach commented Sep 19, 2024

Hmm, still looks off a bit on some of the other renderers. Perhaps we keep the radios as circles, and the checkboxes as non-rounded boxes? 🤔 I did see that some of the parent data is being set in nk_do_toggle().

@awschult002
Copy link
Contributor Author

Hmm, still looks off a bit on some of the other renderers. Perhaps we keep the radios as circles, and the checkboxes as non-rounded boxes? 🤔 I did see that some of the parent data is being set in nk_do_toggle().

My understanding is that the checkbox is drawing a rect_stroke, then the do_toggle is doing a rect_fill of a smaller size to overlap the inside of the checkbox.

But for it still looking "off" on some. The basic XDraw commands on rounded edges are producing the artifacts regardless of the new toggle behaviour.

I think removing the rounded corners for checkboxes would definitely help.

the rounded corners on the checkboxes looked terrible with the X11 and rawfb demos. This was also in-congruent with the previous look. Reverting to sharp corners keeps the previous look while also removing the issue of the X11 looking terrible. 

This commit also shrinks the select toggle box inside the checkbox by a couple pixels to better match the radio button's look and feel
@awschult002
Copy link
Contributor Author

@RobLoach

The corner issues have been fixed. X11 and others now look great. I believe this PR is ready to go.

image

@awschult002
Copy link
Contributor Author

@RobLoach: I believe this PR is good to go. If not, I would love some feedback. I certainly understand that you are busy and can't always get to everything as you would like to. I am curious though, who are the maintainers of this repo? Who are the reviewers? If, as a contributor, I have a feature that I think is influential; where can I get advice or feedback on it?

The best I have is looking through this repo and making comments like the one I am typing up now; but I don't think that is as efficient as can be. What can I do to help with this? Is there some desire to get more people involved? is there some other chat/IRC group that I can get involved with?

I will admit. I love this project, and these PRs that I have posted are some of the only PRs i have ever created. This project being the only one that I have genuinely felt the desire to actually contribute to. I am very interested in seeing this project have much greater success, and I am willing to help out as both a contributor and reviewer if desired.

Please let me know.

@RobLoach
Copy link
Contributor

Thanks for the ping! Been pretty busy over the past month or so but would love to get this update in. I'll try to fix up the conflict.

@awschult002
Copy link
Contributor Author

All conflicts should be resolved

@riri
Copy link
Contributor

riri commented Nov 17, 2024

I get some time lately and even if I didn't participate to evolutions, I kept an offline eye on PR :)
Tested for my main platforms: glfw/opengl3, sdl/opengl3, x11, x11/xft
LGTM, just a little glitch on x11 without xft (the lower right corner is squared) but I don't think we need to block the PR for this

This was referenced Nov 17, 2024
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