Fix DragValue expansion when editing
#5809
Conversation
|
Preview available at https://egui-pr-preview.github.io/pr/5809-drag-value-expansion |
lucasmerlin
left a comment
lucasmerlin
left a comment
There was a problem hiding this comment.
Nice! Seems to work well.
About the snapshots, could you try running the script I added in this PR to update your snapshots (by downloading / copying or merging that branch)?
#5816
| let mut output = self.show_content(ui); | ||
|
|
||
| // TODO(emilk): return full outer_rect in `TextEditOutput`. | ||
| // Can't do it now because this fix is ging into a patch release. | ||
| let outer_rect = output.response.rect; | ||
| let inner_rect = outer_rect - margin; | ||
| output.response.rect = inner_rect; | ||
| let output = self.show_content(ui); | ||
|
|
||
| if frame { | ||
| let visuals = ui.style().interact(&output.response); | ||
| let frame_rect = outer_rect.expand(visuals.expansion); | ||
| let frame_rect = output.response.rect.expand(visuals.expansion); |
There was a problem hiding this comment.
We should mention this as a breaking change in the changelog
There was a problem hiding this comment.
Is that something I should do? And where? CHANGELOG.md?
There was a problem hiding this comment.
No, sorry, I was just commenting this for myself, I'll update the title / description accordingly when merging the PR
MStarha
force-pushed
the
drag-value-expansion
branch
from
7c0eea9 to
4a53f4c
Compare
After |
|
Hmm weird, but thanks for trying! |
crates/egui/src/widgets/button.rs
Outdated
| let font_selection = FontSelection::default(); | ||
| let font_id = font_selection.resolve(ui.style()); | ||
| let row_height = ui.fonts(|f| f.row_height(&font_id)); |
There was a problem hiding this comment.
Hmm, I think this could break though, if the RichText sets a different font?
There is RichText::font_height, I think you could use that instead 👍🏻
There was a problem hiding this comment.
The text is the more generic WidgetText, but the font_height method is still there. Nevertheless, there seems to be a bug in the text layouter that results in different text heights, becase the font height reported by the WidgetText is different than the text height reported by the Galley, that is created from the WidgetText.
* [x] I have followed the instructions in the PR template This PR fixes an issue, where `DragValue` would expand when it enters editing state. There were 3 contributing problems: - A workaround introduced in emilk#4276 caused the `DragValue` to report incorrect outer size. - The `DragValue` uses `TextEdit` internally and sets both `min_size` and `desired_width` to the same value - desired width is used **before** padding is applied - this is in contrast to `Button` (also used internally by `DragValue`), which only uses `min_size`. This caused the `DragValue` to expand horizontally by the size of button padding. - The height of the `TextEdit` is (among other things) determined by the height of the row, which was not present in `Button`. This caused a small vertical expansion, when the contents (including padding) were larger than the `min_size`.  Here the dimensions set in code are: - padding: 20 x 20 pt - interact size: 80 x 30 pt *Note: I do not know what's up with the tests. When I ran the check script, they were failing because of 3 UI missmatches, so I updated the snapshots. Now, the updated snapshots cause the same failure in CI, that appeared locally before the update. Now the locally run tests fail with `The platform you're compiling for is not supported by winit` and couple more following errors coming from the same source (`winit 0.30.7`).*
2 participants
This PR fixes an issue, where
DragValuewould expand when it enters editing state. There were 3 contributing problems:TextEditan atomic widget #4276 caused theDragValueto report incorrect outer size.DragValueusesTextEditinternally and sets bothmin_sizeanddesired_widthto the same value - desired width is used before padding is applied - this is in contrast toButton(also used internally byDragValue), which only usesmin_size. This caused theDragValueto expand horizontally by the size of button padding.TextEditis (among other things) determined by the height of the row, which was not present inButton. This caused a small vertical expansion, when the contents (including padding) were larger than themin_size.Here the dimensions set in code are:
Note: I do not know what's up with the tests. When I ran the check script, they were failing because of 3 UI missmatches, so I updated the snapshots. Now, the updated snapshots cause the same failure in CI, that appeared locally before the update. Now the locally run tests fail with
The platform you're compiling for is not supported by winitand couple more following errors coming from the same source (winit 0.30.7).