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

Use egui::Modal to implement our modal dialogs #8288

Merged
merged 9 commits into from
Dec 5, 2024
Merged

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Dec 3, 2024

Related

What

Our modal used to be implemented using a wrapper which used egui::Window. The guts of that wrapper are now reimplemented based on egui::Modal. That wrapper is now named ModalWrapper. This does not affect "user" code, which can still use the existing ModalHandler.

This PR does not affect the styling of the modals. The only difference is modal are now centred on the window (that egui::Modal's default behaviour). Previously they were slightly offset towards the top of the window.

image

TODO

Copy link

github-actions bot commented Dec 3, 2024

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link
05a3f7e https://rerun.io/viewer/pr/8288

Note: This comment is updated whenever you push a commit.

@abey79 abey79 added ui concerns graphical user interface 🚜 refactor Change the code, not the functionality exclude from changelog PRs with this won't show up in CHANGELOG.md labels Dec 3, 2024
@abey79 abey79 marked this pull request as ready for review December 3, 2024 14:25
@teh-cmc teh-cmc self-requested a review December 4, 2024 08:29
Copy link
Member

Choose a reason for hiding this comment

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

Am I somehow magically looking at LFS-backed data, or was this committed directly to the repo?

Copy link
Member Author

Choose a reason for hiding this comment

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

TL;DR: yes.

GH is extremely bad at displaying that stuff. The best I could do is to go to the relevant branch, navigate to the file in question, and notice the tiny mention top left:

image

Also, CI allegedly checks for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

TIL you can navigate to that file by using the … menu in the diff view and pick "View file".

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

Ditto

@abey79 abey79 merged commit f78bada into main Dec 5, 2024
31 checks passed
@abey79 abey79 deleted the antoine/use-egui-modal branch December 5, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 🚜 refactor Change the code, not the functionality ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants