Fix cursor clipping in TextEdit inside a ScrollArea#3660
Conversation
juancampa
changed the title
scroll_target values
juancampa
changed the title
scroll_target valuesScrollArea now ignores unrelated scroll_target values
|
The reason for this fix btw is because the temporary workaround #153 (cc @Barugon) has a problem. Notice that on the fourth Screen.Recording.2023-11-29.at.10.52.31.AM.mov |
juancampa
force-pushed
the
ignore-unrelated-scroll-targets
branch
from
ff4b722 to
d7bd15d
Compare
juancampa
changed the title
ScrollArea now ignores unrelated scroll_target valuesTextEdit inside a ScrollArea
|
@emilk I just reimplemented this on top of the latest |
|
Preview available at https://egui-pr-preview.github.io/pr/3660-ignore-unrelated-scroll-targets |
| /// If there was a scroll target before the ScrollArea was added this frame, it's | ||
| /// not for us to handle so we save it and restore it after this ScrollArea is done. | ||
| saved_scroll_target: [Option<pass_state::ScrollTarget>; 2], |
There was a problem hiding this comment.
There is also FrameState::scroll_delta, I think we should do the same with that?
There was a problem hiding this comment.
@lucasmerlin There's no FrameState anymore so I think this comment is not valid anymore?
|
thanks! |
|
This change breaks |
|
Also, I haven't been able to reproduce this bug in the current version, but only when I remove the code from lines 726-734 in |
This PR reverts a change introduced in PR #3660 that caused a regression with `TextEdit::singleline`. The original PR attempted to fix an issue with the cursor in `TextEdit` inside `ScrollArea`, but it did so by adding unnecessary size allocation to `TextEdit`, which breaks the layout when `TextEdit::singleline` is used outside of `ScrollArea`.  The regression introduced by #3660 is more severe, as it completely breaks the layout of applications using `TextEdit::singleline`, as shown in the following issues: * Closes #5500 * Closes #5597 Furthermore, I was unable to reproduce the original bug from PR #3660 in the current version of egui using the following code: ```rust impl eframe::App for MyEguiApp { fn update(&mut self, ctx: &egui::Context, _: &mut eframe::Frame) { ctx.set_debug_on_hover(true); egui::CentralPanel::default().show(ctx, |ui| { ScrollArea::vertical().max_height(100.0).show(ui, |ui| { ui.add(TextEdit::multiline(&mut self.text).hint_text("Enter text here...")) }); }); } } ``` This code attempts to recreate the layout shown in the video from PR #3660, using a `ScrollArea` with limited height and a `TextEdit` inside. However, the cursor hiding issue was not reproducible.  Therefore, I believe the code added in PR #3660 is no longer necessary and only creates more problems. * Closes #5500 * Closes #5597 * [x] I have followed the instructions in the PR template
This PR reverts a change introduced in PR #3660 that caused a regression with `TextEdit::singleline`. The original PR attempted to fix an issue with the cursor in `TextEdit` inside `ScrollArea`, but it did so by adding unnecessary size allocation to `TextEdit`, which breaks the layout when `TextEdit::singleline` is used outside of `ScrollArea`.  The regression introduced by #3660 is more severe, as it completely breaks the layout of applications using `TextEdit::singleline`, as shown in the following issues: * Closes #5500 * Closes #5597 Furthermore, I was unable to reproduce the original bug from PR #3660 in the current version of egui using the following code: ```rust impl eframe::App for MyEguiApp { fn update(&mut self, ctx: &egui::Context, _: &mut eframe::Frame) { ctx.set_debug_on_hover(true); egui::CentralPanel::default().show(ctx, |ui| { ScrollArea::vertical().max_height(100.0).show(ui, |ui| { ui.add(TextEdit::multiline(&mut self.text).hint_text("Enter text here...")) }); }); } } ``` This code attempts to recreate the layout shown in the video from PR #3660, using a `ScrollArea` with limited height and a `TextEdit` inside. However, the cursor hiding issue was not reproducible.  Therefore, I believe the code added in PR #3660 is no longer necessary and only creates more problems. * Closes #5500 * Closes #5597 * [x] I have followed the instructions in the PR template (cherry picked from commit e995c4c)
4 participants
Before
Notice how the cursor hides after third enter and when the line is long.
Screenshot.2024-10-13.at.04.36.56.mp4
After
Cursor is always visible
Screenshot.2024-10-13.at.04.37.52.mp4
ScrollAreanow checks if there's ascroll_targetinbegin, if there is, it saves it because it's not from its children, then restore it inend.TextEditnow allocates additional space if its galley grows during the frame. This is needed so that any surroundingScrollAreacan bring the cursor to view, otherwise the cursor lays outside the theScrollArea'scontent_ui.