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

TextCursorState.set_char_range() interacts wrongly with user click #3883

Open
abey79 opened this issue Jan 24, 2024 · 3 comments
Open

TextCursorState.set_char_range() interacts wrongly with user click #3883

abey79 opened this issue Jan 24, 2024 · 3 comments
Labels
bug Something is broken

Comments

@abey79
Copy link
Collaborator

abey79 commented Jan 24, 2024

Considering the repro code below, regardless of how the text edit gains focus, I'm expecting all of the text to be selected. It works as expected when Tabing, or clicking in the text edit whitespace.

However, when clicking on the text itself, only the text up to the mouse cursor is selected.

set_cursor_bug.mp4
use eframe::egui;

fn main() -> Result<(), eframe::Error> {
    env_logger::init(); // Log to stderr (if you run with `RUST_LOG=debug`).
    let options = eframe::NativeOptions {
        viewport: egui::ViewportBuilder::default().with_inner_size([320.0, 240.0]),
        ..Default::default()
    };
    eframe::run_native(
        "My egui App",
        options,
        Box::new(|cc| {
            // This gives us image support:
            egui_extras::install_image_loaders(&cc.egui_ctx);

            Box::<MyApp>::default()
        }),
    )
}

struct MyApp {
    name: String,
}

impl Default for MyApp {
    fn default() -> Self {
        Self {
            name: "Arthur".to_owned(),
        }
    }
}

impl eframe::App for MyApp {
    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
        egui::CentralPanel::default().show(ctx, |ui| {
            ui.heading("My egui Application");
            ui.horizontal(|ui| {
                ui.label("Your name: ");

                let mut output = egui::TextEdit::singleline(&mut self.name).show(ui);

                if output.response.gained_focus() {
                    output.response.request_focus();
                    let min = egui::text::CCursor::new(0);
                    let max = egui::text::CCursor::new(self.name.len());
                    let new_range = egui::text::CCursorRange::two(min, max);
                    output.state.cursor.set_char_range(Some(new_range));
                    output.state.store(ui.ctx(), output.response.id);
                }
            });
        });
    }
}
@abey79 abey79 added the bug Something is broken label Jan 24, 2024
@abey79 abey79 added this to the Next Major Release milestone Jan 24, 2024
abey79 added a commit to rerun-io/rerun that referenced this issue Jan 24, 2024
### What

This PR adds the possibility to edit a space view's space origin.

**Note**:This PR is affected by the following issue:
- emilk/egui#3883

TODO:
- [x] Are we happy with the general behaviour of this thing?
- [x] Should this be feature gated?


https://github.com/rerun-io/rerun/assets/49431240/37571532-0326-4e43-98ee-8d0e537d9235

Follow-up issues:
- #4849

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4848/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4848/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4848/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4848)
- [Docs
preview](https://rerun.io/preview/307d7c3dc6416fde83960c1c6dabda94f26a0425/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/307d7c3dc6416fde83960c1c6dabda94f26a0425/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@emilk
Copy link
Owner

emilk commented Feb 5, 2024

This is because the TextEdit gains focus as soon as the mouse pressed down on it, but the frame after the mouse button is still down, and now TextEdit treats it as a drag-to-select sort of situation.

Not sure how to proceed here

@abey79
Copy link
Collaborator Author

abey79 commented Feb 5, 2024

I guess a workaround would then be a mini state machine to trigger select on the frame after gain_focus...

@abey79
Copy link
Collaborator Author

abey79 commented Feb 5, 2024

This work-around sort-of-works. Nothing is selected so long as the mouse is down and the selection becomes correct on mouse release.

Some/all of these request repaint might be superfluous as mouse state change would trigger the draw anyways.

use eframe::egui;

fn main() -> Result<(), eframe::Error> {
    env_logger::init(); // Log to stderr (if you run with `RUST_LOG=debug`).
    let options = eframe::NativeOptions {
        viewport: egui::ViewportBuilder::default().with_inner_size([320.0, 240.0]),
        ..Default::default()
    };
    eframe::run_native(
        "My egui App",
        options,
        Box::new(|cc| {
            // This gives us image support:
            egui_extras::install_image_loaders(&cc.egui_ctx);

            Box::<MyApp>::default()
        }),
    )
}

struct MyApp {
    name: String,
    focus_gained: bool,
}

impl Default for MyApp {
    fn default() -> Self {
        Self {
            name: "Arthur".to_owned(),
            focus_gained: false,
        }
    }
}

impl eframe::App for MyApp {
    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
        egui::CentralPanel::default().show(ctx, |ui| {
            ui.heading("My egui Application");
            ui.horizontal(|ui| {
                ui.label("Your name: ");

                let mut output = egui::TextEdit::singleline(&mut self.name).show(ui);

                if output.response.gained_focus() {
                    self.focus_gained = true;
                    ctx.request_repaint();
                }

                if self.focus_gained {
                    // wait for the mouse to be released before selecting all
                    if ui.input(|i| !i.pointer.any_down()) {
                        self.focus_gained = false;

                        output.response.request_focus();
                        let min = egui::text::CCursor::new(0);
                        let max = egui::text::CCursor::new(self.name.len());
                        let new_range = egui::text::CCursorRange::two(min, max);
                        output.state.cursor.set_char_range(Some(new_range));
                        output.state.store(ui.ctx(), output.response.id);
                    } else {
                        ctx.request_repaint();
                    }
                }
            });
        });
    }
}

@emilk emilk removed this from the Next Major Release milestone Dec 11, 2024
@emilk emilk added this to egui Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken
Projects
Status: No status
Development

No branches or pull requests

2 participants