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

Remove hover effect in checkboxes #2727

Closed
Tracked by #2670 ...
martenbjork opened this issue Jul 18, 2023 · 3 comments · Fixed by #2796 or #2851
Closed
Tracked by #2670 ...

Remove hover effect in checkboxes #2727

martenbjork opened this issue Jul 18, 2023 · 3 comments · Fixed by #2796 or #2851
Labels
ui concerns graphical user interface

Comments

@martenbjork
Copy link
Contributor

martenbjork commented Jul 18, 2023

When hovered, the checkboxes grow in size. They shouldn't. The scale of the element should remain the same. Only the color should change on hover.

image
@Wumpf Wumpf added the ui concerns graphical user interface label Jul 18, 2023
@abey79
Copy link
Member

abey79 commented Jul 24, 2023

Here is the current situation, which applies to all controls:

{
    // Expand hovered and active button frames:
    egui_style.visuals.widgets.hovered.expansion = 2.0;
    egui_style.visuals.widgets.active.expansion = 2.0;
    egui_style.visuals.widgets.open.expansion = 2.0;
}

Setting hovered.expansion to 0 removes the scaling on hover.

Questions:

  • Should the scaling be removed on click as well (active.expension = 0.0? I would say this is not necessary as there is already some click feedback with thickening of the white check mark.
  • How about open.expension? It refers buttons dropdown menu, see 2nd video.
  • Since this is a rather global setting, do we expect undesirable side-effects (e.g. other types of widgets would should scale on hover and/or activation)?

This is with both hovered and active set to 0.0:

Export-1690193981179.mp4

Effect of open.expansion = 2.0 (current situation):

Export-1690194564613.mp4

@martenbjork
Copy link
Contributor Author

I think scaling is a disorienting pattern. I think we should remove it across the app. Colors + hover cursor should be enough to signify interactivity. So, I think we should remove scaling from any interactive elements for now.

If we remove it, we will spot where states are missing and then we can fix that upstream in the design system 🙂

abey79 added a commit that referenced this issue Jul 24, 2023
abey79 added a commit that referenced this issue Jul 24, 2023
### What

Fixes #2727
Fixes #2726
Fixes #2730

Questions for the reviewers:
- This affects the popup menu controls. Due to their styling the
expansion effect was invisible, but resulted in the greyed hover area to
be larger by 2px all around. The difference is visible in the videos
below. Is that OK? @martenbjork
- This PR removes the expansion effect from the blueprint tree's
triangles, and the play/pause/etc. buttons (desired, not shown in the
videos below). Did I miss some other, undesirable side-effect elsewhere?

Before:


https://github.com/rerun-io/rerun/assets/49431240/9990dc45-0294-4e04-9331-94eccba4a1d7

After:


https://github.com/rerun-io/rerun/assets/49431240/4fa29bf8-e9cf-4224-8c23-3427474576cc

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2796) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2796)
- [Docs
preview](https://rerun.io/preview/pr%3Aantoine%2Fno-expansion-2727/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aantoine%2Fno-expansion-2727/examples)
@abey79 abey79 reopened this Jul 26, 2023
@abey79
Copy link
Member

abey79 commented Jul 26, 2023

#2796 reverted by #2824

abey79 added a commit that referenced this issue Jul 27, 2023
abey79 added a commit that referenced this issue Jul 27, 2023
abey79 added a commit that referenced this issue Jul 28, 2023
### What

This PR migrates our checkboxes and radio_values to a custom
implementation in `re_ui`. Currently, the only change is the removal of
the extension effect. In the future, the customisation might go deeper
(e.g. custom tick mark, a variant for popup menu, etc.), possibly all
the way to copy/pasting and rewriting the entire checkbox code.

This PR also introduces a lint to forbid the use of
`ui.checkbox()`/`ui.radio_value()`.

Fixes #2727

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2851) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2851)
- [Docs
preview](https://rerun.io/preview/pr%3Aantoine%2Fui-custom-checkbox/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aantoine%2Fui-custom-checkbox/examples)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui concerns graphical user interface
Projects
None yet
3 participants