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 set_enabled(true) #4327

Closed
antonWetzel opened this issue Apr 5, 2024 · 4 comments · Fixed by #4614
Closed

Remove set_enabled(true) #4327

antonWetzel opened this issue Apr 5, 2024 · 4 comments · Fixed by #4614
Assignees
Labels

Comments

@antonWetzel
Copy link

Is your feature request related to a problem? Please describe.

set_enabled and set_visible sound self explanatory to me, so I did not read the docs.
I wrote

ui.set_enabled(show_cool_widged);
ui.cool_widged(...);
ui.set_enabled(true);
ui.even_cooler_widged(...);

and wonder why my cooler widged is disabled.

Describe the solution you'd like

Remove (or deprecate) ui.set_enabled(...) and ui.set_visible(...).
Add ui.disable() and ui.hide().

Additional context

No clue why this design was choosen, but I think a function named ui.set_enabled should be able to set the ui to enabled.

@emilk
Copy link
Owner

emilk commented Apr 7, 2024

Good points

@emilk emilk added this to the Next Major Release milestone Apr 7, 2024
@emilk emilk added the egui label Apr 7, 2024
@oscargus
Copy link
Contributor

oscargus commented Apr 8, 2024

The painter has a method set_invisible, is that a preferred name over hide? (Or should one rename set_invisible to hide?)

(The doc-string of set_invisible is probably not up-to-date either, the mentioning of false doesn't make sense

/// If `false`, nothing added to the painter will be visible
pub(crate) fn set_invisible(&mut self) {
)

@emilk
Copy link
Owner

emilk commented Apr 8, 2024

I think set_invisible() is clearer than hide() yes (the latter could be misunderstood as e.g. closing a menu)

@antonWetzel
Copy link
Author

hide was just my first idea, I have no preference for the function name.

@emilk emilk self-assigned this Jun 5, 2024
emilk added a commit that referenced this issue Jun 5, 2024
These were confusing, because `set_enabled(true)` and
`set_visible(true)` did nothing.

Instead use one of:
* `ui.add_enabled`, `ui.add_enabled_ui` or `ui.disable()`
* `ui.add_visible`, `ui.add_visible_ui` or `ui.set_invisible()`

* Closes #4327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants