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

Fix: button_padding when using image+text buttons #2510

Merged
merged 7 commits into from
Jan 23, 2023

Conversation

RobWalt
Copy link
Contributor

@RobWalt RobWalt commented Dec 26, 2022

Changes of the PR:

  • fix button_padding functionality for buttons with images

Only the last comment of this PR is important. All other comments aren't relevant after my self review anymore.


TODO:

  • Unless this is a trivial change, add a line to the relevant CHANGELOG.md under "Unreleased".
  • run cargo fmt
  • run cargo clippy
  • self-review
  • run ./sh/check.sh

Closes #2509.

- add `image_margin` field on `Button` widget
- implement setter method called `image_margin` for `Button` widget
- use margin from `image_margin` field of `Button` widget in `Widget`
  trait functions
@RobWalt
Copy link
Contributor Author

RobWalt commented Dec 26, 2022

Question @Reviewer: Should we maybe extend the margin feature to respect the text aswell, i.e. the right field of the Margin creates a margin on the right side of the text in the button instead?

@RobWalt RobWalt marked this pull request as ready for review December 26, 2022 17:42
@RobWalt
Copy link
Contributor Author

RobWalt commented Dec 26, 2022

Question @Reviewer: Should we maybe extend the margin feature to respect the text aswell, i.e. the right field of the Margin creates a margin on the right side of the text in the button instead?

It seems like this would be the better solution as we already have the icon_spacing to configure the space between the image and the text. I implemented a PoC of the changes above and the example with image_margin(Margin::same(20.0)) and icon_spacing = 30.0; would look as follows:

image

@RobWalt
Copy link
Contributor Author

RobWalt commented Dec 28, 2022

I just noticed that the style option button_padding does the trick. It just wasn't applied to the left of the button when there was an image. Setting the button_padding to Vec2::splat(20.0) in the example above looks like this

before

image

after

image

Note that this makes much more sense, since I also explicitly set icon_spacing = 0.0, which isn't the case in the "before" picture.

@RobWalt RobWalt changed the title Implement image button margin Fix: button_padding when using image+text buttons Dec 28, 2022
@RobWalt RobWalt mentioned this pull request Jan 1, 2023
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Good catch

@emilk emilk merged commit 0ad8aea into emilk:master Jan 23, 2023
lictex pushed a commit to lictex/egui that referenced this pull request Jan 23, 2023
* feat(image-button-margin): implement image button margin

- add `image_margin` field on `Button` widget
- implement setter method called `image_margin` for `Button` widget
- use margin from `image_margin` field of `Button` widget in `Widget`
  trait functions

* feat(image-button-margin): update changelog

* feat(image-button-margin): implement `map_or` clippy fix

* feat(image-button-margin): remove margin field & fix button-padding instead

* feat(image-button-margin): fix CI errors

* feat(image-button-margin): update changelog to include fix

* feat(image-button-margin): re-add changes after creating screenshots for PR
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.

Margin for Image Buttons
2 participants