ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
61.03k stars 10.29k forks source link

Read-only / Disabled widgets #211

Open heroboy opened 9 years ago

andoma commented 9 years ago

+1 on this, currently i just decrease alpha to make it look disabled but widgets still respond to events (obviously). Would be nice to have this built-in

ocornut commented 9 years ago

Which widget in particular are you using it for?

andoma commented 9 years ago

Typically sliders and buttons

image

vs

image

(this is with adjusting global alpha)

ocornut commented 9 years ago

Probably rather low-priority. There's two aspects to it - the interaction and the rendering. The interaction is probably easy to sort out. Outside api to be based on PushButtonRepeat()/PopButtonRepeat() (storing a bool + a stack) and check it in the right spot.

The rendering side is more work because we ought to decide on the design here, how are disabled state represented in general, how it can be part of the theme in a sensible lightweight manner without adding dozens of new entries into the theme system that the user would have to manipulate. Perhaps the theme system can compute certain colors based on HSV space operations and store that so widgets code don't have to do those calculation. e.g. we'd have a Colors[] array and a ColorsDisabled[] array that's auto computed. This tie somehow with some of the ground toward a potential new design (#184).

For now I suppose we could add the feature as affecting interaction only, probably wouldn't hurt but it would have to be documented as "visuals will change in a future version".

andoma commented 9 years ago

Just the interaction-parts is perfectly sufficient for me.

Btw, thanks for all your awesome work on this.

Pagghiu commented 9 years ago

In our software I've been handling the disabled state myself, "wrapping" existing imgui calls.

ocornut commented 9 years ago

Some notes. Are you looking for a "InteractionDisabled" or "EditDisabled/Readonly" flag ?

heroboy commented 9 years ago

I think "Disable" means disable all child widgets including the scrollbar, and preventing all mouse interactive. And only a few widget has "Readonly" flag. Readonly widget can interact as normal, but the content will not be changed. And now the readonly can easily implement in application level, for example:

float tmp = x;
ImGui::InputFloat("xxx",&x);
x = tmp;

I think this is good, because windows controls act like this.

ocornut commented 9 years ago

I think read-only should be global and inherited as well, so you can easily turn an entire block into read-only and still, scroll and copy data. That solution above a bit a hacky and would show an edit flicker before the revert, it's not really satisfying enough.

benvanik commented 9 years ago

@ocornut agreed - having something similar to button repeat would be nice. It'd also prevent the need for a bool disabled/bool read_only/etc on everything.

I was just running into this myself needing both read-only content and separately fully-disabled UI (no input/greyed out/etc). My hacky solution in lieu of a real one is to filter some of the mouse events in broad regions before handing them to imgui and to draw a giant semi-translucent rect over everything at the end. It's nasty :)

ocornut commented 9 years ago

Alright,

I have added a ImGuiInputTextFlags_ReadOnly flag to InputText() / InputTextMultiline() as an exception because I think it is a more common use case (to make blobs of text more naturally copiable, even though you can use the logging api for that too).

We need different flag for Disabled and global- ReadOnly. Earlier is easier to implement, later probably a bit more work, as we need to dig a little more inside widgets to make sure things like IsItemHovered() works but the widget doesn't react. So probably will add Disabled early on. It will have to be documented as "unfinished" because it won't affect the visuals just you but it WILL in the future.

ocornut commented 9 years ago

There's another usage pattern which I found useful in one app I made, which is to have widgets that looks disabled but are active. Here I have a set of variables that can be overriden. You can click the override checkbox but you can also directly click on disabled-lookin widget on the right-side and it automatically set the override flag as you click them. That's easily done with just changing text color.

disabled-override

Probably won't affect this task here, but I wanted to share that it may be also a viable idea in some cases.

ocornut commented 9 years ago

@haddock07 in #330

Is there a way to disable a widget (shown but with a disabled state)? For example, I would like to have a check box, in a disabled state, where it couldn't be activated or hovered, and displayed with a darker color. Of course, the function ImGui::Checkbox would always return false. I did a workaround by setting the same color for the 3 states (normal, hovered, active), but when the user clicks on the check box, the check mark is actually shown one frame before returning to the unchecked state.

r-lyeh-archived commented 8 years ago

Hey,

I was looking for possible CheckboxDisabled()/RadioDisabled()/MenuItemDisabled() widgets and found this thread instead. I guess I may share some feedback.

Rather than multiplexing the workload into infinite disabled widget variants that may be (very) time consuming, my suggestion would be rather to create a single BeginDisabled()/EndDisabled(); group that pushes disable state into the stack. From this point:

a) I would expect to be able to browse still the disabled group (scrollbars and tooltips would be active), but not able to interact with any variable modifier at all (checkboxes, selectables, combos, input boxes and so on). Ie, all sub-widgets that may change the user variables shall not react to keyboard/mouse events (ever); any other will. So all my widget values are safe (constant) inside a disabled stack.

b) I would expect the rendered widgets would look similar to a blended widget+background. Achieved either by blending every sub-widget with alpha *0.5, or by doing fancy HSV to blend every rendered color with the background color.

Keep up the good work!

Edit: PS: you could store all the disabled colors variants (the (HSV+color background)/2 thing) when applying styles, so everything would be "cached" in anticipation.

teamimx commented 8 years ago

+1 on the general idea of a disabled state :)

leftspace89 commented 7 years ago

https://github.com/ocornut/imgui/compare/master...leftspace89:master

thx for information :)

ratzlaff commented 7 years ago

@leftspace89 By chance, could you make a patch instead of a series of images?

sonoro1234 commented 7 years ago

+1 on the general idea of a disabled state :)

qimmer commented 7 years ago

+1 on the general idea of a disabled state

codecat commented 7 years ago

Any update on this?

ldecarufel commented 7 years ago

I'd just like to chime in and say that I also would like a way to disable groups of controls.

Maybe PushDisabledState()/PopDisabledState() functions would be the best, since it follows ImGui's stack-based approach.

Maybe disabled controls could simply be drawn in the monochrome version of their regular color, multiplied by a predefined color in the styles.

r-lyeh-archived commented 7 years ago

I just hacked it from ideas in this thread. Hovering, tooltips and input texts work as expected when disabled.

namespace ImGui {
    std::vector<float> alphas;
    void PushDisabled( bool disabled = true ) {
        alphas.push_back( GetStyle().Alpha );
        GetStyle().Alpha = disabled ? 0.25f : 1.0f;
    }
    void PopDisabled( int num = 1 ) {
        while( num-- ) {
            GetStyle().Alpha = alphas.back();
            alphas.pop_back();
        }
    }
    // Install: edit ImGui::IsMouseHoveringRect() and change return to:
    //   return rect_for_touch.Contains(g.IO.MousePos) && (GetStyle().Alpha >= 1);
}

image

ocornut commented 7 years ago

I have pushed the simplest form of disabled flag, but haven't exposed it (it's in imgui_internal.h for now) It doesn't affect the style but you can apply an alpha multiplier fairly easily. (Btw @r-lyeh why are you using your own stack and not PushStyleVar which does exactly that?)

    static bool disabled = false;
    ImGui::Checkbox("Disable", &disabled);
    if (disabled)
    {
        ImGui::PushItemFlag(ImGuiItemFlags_Disabled, true);
        ImGui::PushStyleVar(ImGuiStyleVar_Alpha, ImGui::GetStyle().Alpha * 0.5f);
    }
    [...]
    if (disabled)
    {
        ImGui::PopItemFlag();
        ImGui::PopStyleVar();
    }

disabled

r-lyeh-archived commented 7 years ago

ah, i overlooked the api :)

Cwiiis commented 6 years ago

I like the solution using PushItemFlag, but it would be nice to be able to use it like you can style flags, so you could push a flag that causes items to be disabled before creating a window. If you push the item flags before creating a window now, you end up creating the debug window instead of affecting subsequent items.

ocornut commented 6 years ago

@Cwiiis the existing other settings (ItemWidth, TextWrapPos) are also using a per-window stack. What's is your use case? Is it just the lack of symmetry that is a problem here? If it was a cross-window setting you may expect the window itself to be disabled?

It is tempting to consider the possibility to move more of those to a shared stack. ItemWidth however makes less sense in a shared stack since it needs a suitable default per-window and values are more likely to be based on window width.

Cwiiis commented 6 years ago

In my use-case, I have a UI where in its 'mobile' mode, there is one main view, then a series of smaller preview windows. When you press the preview windows, they become the main view (and the main view shrinks and joins the preview windows). One of these views is a control-panel and I disable controls in it so that you can't accidentally activate anything until it's a main view.

The small issue is that without being able to disable items across windows, I need to pass a flag around a few different functions when the code that creates these windows isn't really concerned with whether or not it's disabled.

It's a good point about what the behaviour of a disabled window should be though - maybe you shouldn't be able to move it? (this particular application disables all window movement anyway, but I could imagine that being useful)

aerydna commented 6 years ago

any update on when this feature will be exposed ? it would be very useful. Anyway thanks for all the work you do!

IanButterworth commented 5 years ago

Has this functionality made it into 1.70? I noticed a mention in the release notes of the gamepad functionality that was added in https://github.com/ocornut/imgui/commit/a26085ed53d07603a9fde0e164ad69773d119642 and referenced above

x1nixmzeng commented 5 years ago

Bumping for interest.

My usage is a row of several buttons I want to show on the debug window at all times:

if (ImGui::Button("Button 1")) { ... }
ImGui::SameLine();
if (ImGui::Button("Button 2")) { ... }

Exposing ButtonEx and ImGuiButtonFlags_Disabled does the trick, but remains part of imgui_internal and doesn't change the style.

ocornut commented 5 years ago

This is still only in the private api because it doesn't affect styling. In the meanwhile you may use .e.g PushStyleVar(ImGuiStyleVar_Alpha, 0.6f) / PopStyleVar() to alter the transparency of those widgets.

andres-asm commented 4 years ago

This is still internal only right? Asking because I use cimgui and I noticed I can't access PushItemFlag

ocornut commented 4 years ago

This is still internal only right?

Right.

cimgui should now include all internals (in the SAME cimgui.h file instead of a separate cimgui_internal.h file which I believe is a critical design flaw).

timbeaudet commented 4 years ago

Bumping for more interest in this becoming exposed, specifically with RadioButtons at the moment but it would be nice to add the ReadOnly flag much like you can with text/input boxes (InputFloat) for instance.

PazerOP commented 3 years ago

Is this something that could potentially be resolved with a pull request? Or is it waiting on love that only ocornut can give?

flamendless commented 3 years ago

Any update on this?

I have pushed the simplest form of disabled flag, but haven't exposed it (it's in imgui_internal.h for now) It doesn't affect the style but you can apply an alpha multiplier fairly easily. (Btw @r-lyeh why are you using your own stack and not PushStyleVar which does exactly that?)

    static bool disabled = false;
    ImGui::Checkbox("Disable", &disabled);
    if (disabled)
    {
        ImGui::PushItemFlag(ImGuiItemFlags_Disabled, true);
        ImGui::PushStyleVar(ImGuiStyleVar_Alpha, ImGui::GetStyle().Alpha * 0.5f);
    }
    [...]
    if (disabled)
    {
        ImGui::PopItemFlag();
        ImGui::PopStyleVar();
    }

disabled

this doesnt work as the PushItemFlag and PopItemFlag are not exposed publicly

bkaradzic commented 3 years ago

This is my hack, compiled from ideas from here...


namespace ImGui
{
    inline void PushEnabled(bool _enabled)
    {
        extern void PushItemFlag(int option, bool enabled);
        PushItemFlag(1<<2 /*ImGuiItemFlags_Disabled*/, !_enabled);
        PushStyleVar(ImGuiStyleVar_Alpha, ImGui::GetStyle().Alpha * (_enabled ? 1.0f : 0.5f) );
    }

    inline void PopEnabled()
    {
        extern void PopItemFlag();
        PopItemFlag();
        PopStyleVar();
    }

} // namespace ImGui
ocornut commented 3 years ago
PushItemFlag(1<<2 /*ImGuiItemFlags_Disabled*/, !_enabled);

Please note we don't provide ABI compatibility and flags occasionally get shuffled or changed value. Best to use imgui_internal.h and you can restrict it to a single cpp file if helpful.

melMass commented 3 years ago

Best to use imgui_internal.h and you can restrict it to a single cpp file if helpful.

I never wanted to open a specific issue for this, but it seems appropriate here, as I'm using it for this reason among others:

What are the reasons against using imgui_internal? I'm using it all over the place in custom widgets without too much issue updating it along with imgui versions. Is this simply a warning because things a more prone to change than in imgui.h?

ocornut commented 3 years ago

Is this simply a warning because things a more prone to change than in imgui.h?

@melMass Yes pretty much.

ocornut commented 3 years ago

Hello all,

While I don't have a 100% satisfying solution yet (as in my mind it ties closely to a rework of the styling system), this issue has been open for ridiculously long and to mitigate this I have at least added the PushDisabled() / PopDisabled() functions in imgui_internal.h (which potentially upgrading them to imgui.h in the future).

1) in the event you have function with same name in same namespace you'll get a link error 2) in the event you used custom functions I would like to encourage you to use those functions now and let me know if they are not a good fit for you.

Thanks!

// PushDisabled()/PopDisabled()
// - Those are not yet exposed in imgui.h because we are unsure of how to alter the style in a way that works for everyone.
//   We may rework this. Hypothetically, a future styling system may set a flag which make widgets use different colors.
// - Feedback welcome at https://github.com/ocornut/imgui/issues/211
// - You may trivially implement your own variation of this if needed.
//   Here we test (CurrentItemFlags & ImGuiItemFlags_Disabled) to allow nested PushDisabled() calls.
void ImGui::PushDisabled()
{
    ImGuiContext& g = *GImGui;
    if ((g.CurrentItemFlags & ImGuiItemFlags_Disabled) == 0)
        PushStyleVar(ImGuiStyleVar_Alpha, g.Style.Alpha * 0.6f);
    PushItemFlag(ImGuiItemFlags_Disabled, true);
}

void ImGui::PopDisabled()
{
    ImGuiContext& g = *GImGui;
    PopItemFlag();
    if ((g.CurrentItemFlags & ImGuiItemFlags_Disabled) == 0)
        PopStyleVar();
}

e.g. image image

ocornut commented 3 years ago

A few people have liked the message above about PushDisabled()/PopDisabled(). I would be interested in your feedback (whereas the Alpha multiply gives a satisfying look with your style) to see if we can promote this to imgui.h in the future.

codecat commented 3 years ago

Not sure if it has been suggested yet, but would a function SetNextWidgetEnabled make sense? Thinking about conditionally enabling (or disabling) certain widgets. Perhaps even passing the enabled/disabled state to PushDisabled (or PushEnabled then?) would be an improvement here:

ImGui::PushEnabled(canUseButton);
if (ImGui::Button("Do something")) {
  // ...
}
ImGui::PopEnabled();

Versus the double conditional test currently required:

if (!canUseButton) ImGui::PushDisabled();
if (ImGui::Button("Do something")) {
  // ...
}
if (!canUseButton) ImGui::PopDisabled();
codecat commented 3 years ago

As for the alpha, in my style it might be a little difficult to see that buttons are actually disabled:

image

But I'm sure I'd be able to make this more pronounced as mentioned in the code's comments, so this is not a huge deal to me. In your screenshots it looks great, imo.

ocornut commented 3 years ago

Not sure if it has been suggested yet, but would a function SetNextWidgetEnabled make sense?

It would make sense to have a SetNextItemEnabled(). Currently it's a little difficult to implement because we don't have standardized "footer" code for all widgets so I can't implement it asap.

Perhaps even passing the enabled/disabled state to PushDisabled (or PushEnabled then?) would be an improvement here:

Interesting. You could wrap it yourself with a SetEnabled(bool v) function I suppose?

As for the alpha, in my style it might be a little difficult to see that buttons are actually disabled: [....] But I'm sure I'd be able to make this more pronounced as mentioned in the code's comments,

Yes and no, my question ties to this specifically. We don't want you to have to modify this value in the code. So I guess we should expose it in ImGuiStyle at the same time we expose PushDisabled() in imgui.h.

ocornut commented 3 years ago

You could wrap it yourself with a SetEnabled(bool v) function I suppose?

Scrap that, a SetEnabled(bool) api would create too much confusion with the stack-based version. What if we did PushEnabled(false) / SetEnabled() ?

I looked at the suggested boolean, PushEnabled(bool), It's a little difficult to apply alpha when nested:

PushEnabled(false);
PushEnabled(true);
// What is value of Alpha at this point? how can we restore "full alpha"
PushEnabled(false);
PushEnabled(true);
PushEnabled(false);
// Ditto

This would require use or store a "CurrentAlpha" (== Alpha * (Disabled? DisabledAlpha : 1.0f))

We could decide that PushEnabled(false) + PushEnabled(true) would leave us disabled (aka Disabled spread to the whole stack) which becomes easier to implement but probably not obvious to user.

So I am tempted to try to figure out how to implement the SetNextItemXXX version first...

Without those changes, tentative commit (in features/disabled branch): https://github.com/ocornut/imgui/commit/0fc170a9cf729c4d6d04af7a10f231edec8ed876

Legulysse commented 3 years ago

Hi !

For reference, the most satisfying "disable" mechanic I have used in production code is the BeginDisabledGroup(bool) / EndDisabledGroup from Unity : https://docs.unity3d.com/ScriptReference/EditorGUI.BeginDisabledGroup.html It brings the benefits pointed out by codecat above (PushEnabled(bool) / PopEnabled), in the sense that the bool parameter allows for more straightforward blocks of code by avoiding a lot of if() statements when surrounding widgets that may become disabled depending on some context.

When using nested calls, as soon as a "disable" state is pushed, everything down the line will stay disabled. So in your example, PushEnabled(false) + PushEnabled(true) would indeed maintain the "disable" state.

The main benefit is that generally, when you want to disable a big chunk of interface, you don't want a random call inside to re-enable a field in the middle of it. I saw this problem happening a lot when we used manual calls on the static GUI.enabled flag. Swapping to Begin/End groups everywhere fixed a lot of stuff and made our tools much more stable.

Considering all this, if I had the choice between using a SetNextItemXXX and blocks of PushEnabled(bool) / PopEnabled, I would use the push/pop 100% of the time.

ocornut commented 3 years ago

Thanks for the feedback. I am reworking the API to take a bool, and you are right it makes more sense that one "disabled" in the stack should always be enough to enforce things (which incidentally is easier to implement, so fine by me!)

My only issue is that bool disabled = true goes the opposite way of BeginMenu()/MenuItem() using bool enabled = true BUT I've simultaneously been very eager to remove the bool parameters in the menu functions, so I am also looking at this whole thing together.

Legulysse commented 3 years ago

My only issue is that bool disabled = true goes the opposite way of BeginMenu()/MenuItem() using bool enabled = true BUT I've simultaneously been very eager to remove the bool parameters in the menu functions, so I am also looking at this whole thing together.

I personally wouldn't mind the logic difference since the function name is very explicit on its behaviour and I always double check parameter names, even though consistency is important. If you plan to remove the menu items in the long run, having some inconsistency seems very acceptable. As a side note, I consider menus and blocks of widgets as two very distinct contexts (menu items will probably have very granular disable conditions, widgets will probably be handled in groups), so having separate mechanisms doesn't feel like its a problem.

You could also go with PushEnabled(bool enabled = true) instead, but I believe it would be counter-intuitive when it comes to the stacking mechanism since the name would suggest a priority over the disabled state, whereas PushDisabled is way more explicit about the "disabled" state always having priority.

ocornut commented 3 years ago

Thanks! Was about to post my message and yours just arrived so this duplicate some of your thoughts.


For BeginMenu/MenuItem: the value for enabled is frequently computed inline at the call site and so it makes some sense that it stays an individual bool rather than named flag:

MenuItem("Property", ..., has_selection);
MenuItem("Property", ..., selected_id == object_id);

For which enabled_is_true logic also makes sense.

Problem is a transition between enabled_is_true item logic to disabled_is_true block logic would be annoying. So in theory it would be nice is the new API could use enabled_is_true logic but I mean this is super bizarre:

BeginEnabledGroup(false)

I've spent 30 minutes browsing the internet trying to find a better noun or verb that would allow enable state use the true value (enablement ? enabledness ?!) but can't find a satisfying one.... BeginEnablementGroup(), PushEnablement() ?

menu items will probably have very granular disable conditions, widgets will probably be handled in groups), so having separate mechanisms doesn't feel like its a problem.

I guess you are right about this.

FYI we also use a _Disabled flag for:

Both Selectable and Button ones are item override essentially the same thing we are dealing with, and they primarily exist because this new API we're working on didn't exist. If we had an hypothetical SetNextItemEnabled(false) API it would work everywhere and not require the existence of widget-specific ImGuiSelectableFlags_Disabled/ImGuiButtonFlags_Disabled.

Also torturing myself on whether to use BeginXXX() vs PushXXX() which got me into revaluating some of the wording choice in the API.