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

Introduce a "Selectable Toggle" widget and use it for the 3D view's camera kind #7064

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Aug 5, 2024

What

This PR introduces UiExt::selectable_toggle(), which builds on Ui::selectable_{label|value} to make a horizontal, toggle-like widget:

let mut flag = false;
ui.selectable_toggle(|ui| {
    ui.selectable_value(&mut flag, false, "Inactive");
    ui.selectable_value(&mut flag, true, "Active");
});

Now used for the 3D view's camera kind selection:

image

Also fix a crash in re_ui_example.

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

@abey79 abey79 added ui concerns graphical user interface include in changelog labels Aug 5, 2024
@abey79 abey79 requested a review from gavrelina August 5, 2024 17:10
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

neat

@gavrelina
Copy link
Member

Super cool!
I have 3 small visual improvements:

  1. the alignment of the text in the box is visually off (not centered optically, see screenshot example, it's just 1-2 px)
  2. the font color of the item that is not selected shall not be the same bright, it shall be less contrast, so please use the same shade as you use for 'camera' or 'coordinates' (in your screenshot)
  3. the grey is not the grey 😬, it's warmer shade, and we shall use colder one. I propose to use the same grey as you'd use for the secondary label (same as for 'camera' or 'coordinates'). It will especially look better once you change the font color.
Screenshot 2024-08-06 at 11 50 03

@abey79
Copy link
Member Author

abey79 commented Aug 6, 2024

For (1), the bad news it that this is a egui issue. The good news is that it's caused by an unaligned cursor coordinate, which I can fix. This makes me want a debug tool for this: emilk/egui#4927

Edit: actually it's not only that. egui seems to have an imperfect text-in-box alignment—not sure how to fix it (font dependant?)

emilk/egui#4929

@abey79 abey79 merged commit 2613431 into main Aug 6, 2024
34 checks passed
@abey79 abey79 deleted the antoine/selectable-toggle branch August 6, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants