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

text cursor blink in TextEdit #4121

Closed
wants to merge 73 commits into from
Closed

Conversation

rustbasic
Copy link
Contributor

text cursor blink in TextEdit

crates/egui/src/widgets/text_edit/builder.rs Outdated Show resolved Hide resolved
crates/egui/src/text_selection/visuals.rs Outdated Show resolved Hide resolved
crates/egui/src/widgets/text_edit/builder.rs Outdated Show resolved Hide resolved
@rustbasic rustbasic requested a review from emilk March 13, 2024 16:10
@rustbasic
Copy link
Contributor Author

@emilk

It's changes were completed.

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.

This is feeling much better now!

Comment on lines 728 to 738
/// show where the text cursor would be if you clicked
pub text_cursor_preview: bool,

/// set the text cursor to blink
pub text_cursor_blink: bool,

/// set the text cursor on duration
pub text_cursor_on_duration: f64,

/// set the text cursor off duration
pub text_cursor_off_duration: f64,
Copy link
Owner

Choose a reason for hiding this comment

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

I think it's time to group all these in a struct TextCursorStyle

Copy link
Owner

Choose a reason for hiding this comment

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

…but I can do that in a later PR

@@ -1846,6 +1862,15 @@ impl Visuals {
ui.checkbox(text_cursor_preview, "Preview text cursor on hover");
ui.add(Slider::new(clip_rect_margin, 0.0..=20.0).text("clip_rect_margin"));

ui.add(Slider::new(resize_corner_size, 0.0..=20.0).text("resize_corner_size"));
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
ui.add(Slider::new(resize_corner_size, 0.0..=20.0).text("resize_corner_size"));

Comment on lines 105 to 113
.request_repaint_after(std::time::Duration::from_millis(
(on_duration * 1000.0) as u64,
));
}
if !is_cursor_visible {
ui.ctx()
.request_repaint_after(std::time::Duration::from_millis(
(off_duration * 1000.0) as u64,
));
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
.request_repaint_after(std::time::Duration::from_millis(
(on_duration * 1000.0) as u64,
));
}
if !is_cursor_visible {
ui.ctx()
.request_repaint_after(std::time::Duration::from_millis(
(off_duration * 1000.0) as u64,
));
.request_repaint_after(std::time::Duration::from_secs_f32(
on_duration
));
}
if !is_cursor_visible {
ui.ctx()
.request_repaint_after(std::time::Duration::from_secs_f32(
off_duration
));

Comment on lines +77 to +78
/// Paint text cursor.
pub fn paint_text_cursor(
Copy link
Owner

Choose a reason for hiding this comment

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

fn paint_text_cursor and fn paint_cursor have very similar names, but one does blinking, and one does not. We should document that. fn paint_cursor should probably be made non-pub too

primary_cursor_rect: Rect,
is_stay_cursor: bool,
) {
let i_time = ui.input(|i| i.time);
Copy link
Owner

Choose a reason for hiding this comment

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

Currently the cursor still blinks while typing, which is quite annoying. If we instead reset a timer each time the user presses a key, it won't blink until text_cursor_on_duration after the user last stopped typing.

So, I think you were right to put this timer in TextEditState. We could put a last_edit_time: Option<f64> there fed by ui.input(|i| i.time), and then paint_text_cursor is fed with time_since_last_edit which will be used as the basis for the animation

@rustbasic rustbasic requested a review from emilk March 29, 2024 17:54
@emilk
Copy link
Owner

emilk commented Mar 30, 2024

I'll fix the rest of the things in this PR

@rustbasic
Copy link
Contributor Author

I'll fix the rest of the things in this PR

Yes, Thank you emilk.

@emilk emilk closed this in #4279 Mar 30, 2024
emilk added a commit that referenced this pull request Mar 30, 2024
On by default. Can be set with `style.text_cursor.blink`.

* Closes #4121
@rustbasic rustbasic deleted the patch19 branch April 22, 2024 10:09
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
On by default. Can be set with `style.text_cursor.blink`.

* Closes emilk#4121
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants