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

Improve web text agent #4561

Merged
merged 8 commits into from
May 29, 2024
Merged

Improve web text agent #4561

merged 8 commits into from
May 29, 2024

Conversation

jprochazk
Copy link
Collaborator

@jprochazk jprochazk commented May 28, 2024

This PR improves the text agent to make fewer assumptions about how egui is embedded into the page:

  • Text agent no longer sets the canvas position
  • There is now a text agent for each instance of WebRunner
  • The input element is now moved to the correct position, so the OS can display the IME window in the correct place. Before it would typically be outside of the viewport

The best way to test this is to build & server the web demo locally:

scripts/build_demo_web.sh && scripts/start_server.sh

Then open the EasyMark editor, and try using IME to input some emojis: http://localhost:8888/#EasyMarkEditor

To open the emoji keyboard use:

  • win + . on Windows
  • ctrl + cmd + space on Mac

Tested on:

  • Windows
  • Linux
  • MacOS
  • Android
  • iOS

Migration guide

The canvas no longer controls its own size/position on the page. This means that those properties can now be controlled entirely via HTML and CSS, and multiple separate eframe apps can coexist better on a single page.

To match the old behavior, set the canvas width and height to 100% of the body element:

<html>
  <body>
    <canvas></canvas>
  </body>
</html>
/* remove default margins and use full viewport */
html, body {
  margin: 0;
  width: 100%;
  height: 100%;
}

canvas {
  /* match parent element size */
  width: 100%;
  height: 100%;
}

Note that there is no need to set position: absolute/left: 50%; transform: translateX(-50%)/etc., and setting those properties may poorly affect the sharpness of egui-rendered text.

Because eframe no longer updates the canvas style in any way, it also means that on mobile, the canvas no longer collapses upwards to make space for a mobile keyboard. This should be solved in other ways: #4572

@jprochazk jprochazk added web Related to running Egui on the web eframe Relates to epi and eframe text Problems related to text IME labels May 28, 2024
@Mm2PL
Copy link

Mm2PL commented May 28, 2024

I was asked to test this, works decently with ibus however there is a slight issue with the input being selected.
When I click-drag my mouse out of the input (like in selecting for example): I cannot use IME.
https://github.com/emilk/egui/assets/25011746/d88877ea-72b5-4c9b-8dc8-f87e4dfada00
In the video the Firefox devtools open because Ctrl+Shift+E isn't stolen by ibus from Firefox.
I tested master and it seems that the issue is present there too.

@jprochazk
Copy link
Collaborator Author

jprochazk commented May 28, 2024

I was asked to test this, works decently with ibus however there is a slight issue with the input being selected.

Thanks! I opened an issue for that:

@jprochazk
Copy link
Collaborator Author

At the very least there don't seem to be any regressions here, and it's definitely improved some things, but also revealed just how broken input handling is on mobile web.

@jprochazk jprochazk marked this pull request as ready for review May 28, 2024 19:57
@emilk
Copy link
Owner

emilk commented May 29, 2024

Tested on iOS Safari, and when I click a text field that is in the lower half of the screen, the on-screen keyboard will now cover that text field, making it impossible to see what I'm typing. On master, the egui canvas would temporarily move up to prevent this problem.

So this is a regression that we should note down in an issue, ideally together with some ideas on how to solve it.

Still, I think a regression on mobile text input (which is already pretty broken anyways) is worth it in the interest of improving desktop web.

Related:

@emilk
Copy link
Owner

emilk commented May 29, 2024

I'm testing bringing up the emoji editor on Mac on desktop chromium, and it will only sometimes track the text field I have selected, but quite often go to the top-left corner. And once it has ended up in the top-left once, it doesn't seem like it wants to leave.

I am testing using the TextEdit field in the WidgetGallery

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.

So nice to get this cleaned up ❤️

But please open an issue describing the regression on mobile.

Are there any other likely regressions?

crates/eframe/src/web/text_agent.rs Outdated Show resolved Hide resolved
crates/eframe/src/web/text_agent.rs Outdated Show resolved Hide resolved
@emilk emilk changed the title Improve text agent Improve web text agent May 29, 2024
@jprochazk
Copy link
Collaborator Author

Opened a bunch of issues and added them to

@jprochazk jprochazk merged commit 514ee0c into master May 29, 2024
35 checks passed
@jprochazk jprochazk deleted the fix-ime branch May 29, 2024 10:54
@emilk emilk added the bug Something is broken label Jun 27, 2024
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
- Closes emilk#4060 - no longer aligned
to top
- Closes emilk#4479 - `canvas.style` is
not set anywhere anymore
- Closes emilk#2231 - same as emilk#4060
- Closes emilk#3618 - there is now one
`<input>` per `eframe` app, and it's removed transitively by
`WebRunner::destroy -> AppRunner::drop -> TextAgent::drop`

This PR improves the text agent to make fewer assumptions about how
`egui` is embedded into the page:
- Text agent no longer sets the canvas position
- There is now a text agent for each instance of `WebRunner`
- The input element is now moved to the correct position, so the OS can
display the IME window in the correct place. Before it would typically
be outside of the viewport

The best way to test this is to build & server the web demo locally:
```
scripts/build_demo_web.sh && scripts/start_server.sh
```

Then open the EasyMark editor, and try using IME to input some emojis:
http://localhost:8888/#EasyMarkEditor

To open the emoji keyboard use:
- <kbd>win + .</kbd> on Windows
- <kbd>ctrl + cmd + space</kbd> on Mac

Tested on:
- [x] Windows
- [x] Linux
- [x] MacOS
- [x] Android
- [x] iOS

## Migration guide

The canvas no longer controls its own size/position on the page. This
means that those properties can now be controlled entirely via HTML and
CSS, and multiple separate `eframe` apps can coexist better on a single
page.

To match the old behavior, set the `canvas` width and height to 100% of
the `body` element:

```html
<html>
  <body>
    <canvas></canvas>
  </body>
</html>
```

```css
/* remove default margins and use full viewport */
html, body {
  margin: 0;
  width: 100%;
  height: 100%;
}

canvas {
  /* match parent element size */
  width: 100%;
  height: 100%;
}
```

Note that there is no need to set `position: absolute`/`left: 50%;
transform: translateX(-50%)`/etc., and setting those properties may
poorly affect the sharpness of `egui`-rendered text.

Because `eframe` no longer updates the canvas style in any way, it also
means that on mobile, the canvas no longer collapses upwards to make
space for a mobile keyboard. This should be solved in other ways:
emilk#4572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken eframe Relates to epi and eframe IME text Problems related to text web Related to running Egui on the web
Projects
None yet
3 participants