-
Notifications
You must be signed in to change notification settings - Fork 570
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
Allow text alignment in button_text_image to be derived from nk_button_style text_alignment #674
base: master
Are you sure you want to change the base?
Allow text alignment in button_text_image to be derived from nk_button_style text_alignment #674
Conversation
44a5e68
to
07c7f65
Compare
Thanks for cleaning this up. Are there any opportunities in the overview demo to show this off or just the demo with the image? Does it improve the SYMBOL usage? If not that's no problem. |
@RobLoach Are you ok with the decision I made for the spacing between the image/symbol and text? I have used half of the distance between the left side of the image/symbol and the left bounds of the widget (and halfway from the right of the image/symbol to the right bounds on the other side). These seem to give enough 'breathing space' around the text, but 1 * the distance might look more balanced. |
@RobLoach Do you want me to prepare anything else here? |
nk_style_push_flags(ctx, &ctx->style.button.text_alignment, NK_TEXT_LEFT); | ||
nk_button_symbol_label(ctx, NK_SYMBOL_TRIANGLE_LEFT, "prev", NK_TEXT_RIGHT); | ||
nk_style_pop_flags(ctx); |
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.
Overall, I think this looks great! My only worry is that when reading it, we set text alignment to left, and then text alignment to right. While I understand why, it does seem confusing.
Would really appreciate more 👀 from others around here before we merge.
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.
Yeah, I can appreciate that. It is actually possible to use the NK_WIDGET_LEFT and NK_WIDGET_RIGHT here - since they are the same values - perhaps that would actually be better. But in that case we would be flipping the meaning if we try to follow what nk_checkbox_label_align
etc uses. Where the WIDGET_ALIGN specifies the position of the checkbox, not the text. Currently nk_button_symbol_label and nk_button_image_label align flags control the text position relative to the symbol, rather than the symbol position relative to the text.
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.
Good point... Perhaps with the flags, that could make sense 🤔
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.
I was trying to avoid introducing new APIs - or changing the behaviour of the existing ones - but one possibility is to introduce nk_button_image_label_align
, nk_button_image_text_align
, and the symbol variants - and in that one provide a widget alignment (that follows the checkbox_align) and text alignment?.
If I do that I think it would probably be a good idea to leave the changes I have made in, as that is just removing the hardcoded CENTER anyway, so is a requirement of the new apis to be able to work.
Co-authored-by: Rob Loach <[email protected]>
Co-authored-by: Rob Loach <[email protected]>
This removes the hardcoded NK_TEXT_CENTERED within a image+text button (eg. menu item or button_image_label etc) and allows the text alignment of text to be specified independently of the relative order of the image and text.
This allows the text to be to the left or right of the image, and allows the text within that content region to then be aligned left/center/right by pushing/setting a button style text_alignment value.
The existing
align
parameter of button_image_label remains and controls the position of the text relative to the image as it did previously. The text alignment itself is controlled with the button style text_alignment flag.For nk_button image label this would be flags
ctx.style.button.text_alignment
For menu items/combo items this would be flags
ctx.style.contextual_button.text_alignment
Example code: (code is in C3 language, using my own bindings, so may look a little odd!)
This also fixes a layout quirk where text in these buttons is aligned centrally within the button but not within the 'remaining' content region.
Before:
After:
This change doesn't otherwise seem to have any negative impact on the example: