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

add [disabled] to submenu #86

Merged
merged 3 commits into from
Feb 12, 2019
Merged

add [disabled] to submenu #86

merged 3 commits into from
Feb 12, 2019

Conversation

klingo
Copy link
Contributor

@klingo klingo commented Jan 27, 2019

This change does:

  • add the "disabled" entry to reference a function for disabling the submenu (like other widgets)
  • in case "disabled" is true, change the submenu label colour to a greyed out version (and reverse)

This change does NOT:

  • disable any child-widgets that belong to the submenu
  • prevent the submenu from collapsing/expanding
  • close the expanded submenu when it is disabled

Only "disabled" the label of the submenu (greyed out text), but does not disable any child widgets nor does it prevent the submenu from expanding/collapsing. Should primarily help to visualise that a whole submenu can be ignored.
@sirinsidiator
Copy link
Owner

Hi. Thanks for the contribution and sorry for not answering earlier. I will review this as soon as I can and hope to upload a new LAM version before Wrathstone goes live.

Regarding your description, wouldn't it be desirable to close and prevent the submenu from opening when it is disabled?

@sirinsidiator sirinsidiator added this to the r27 milestone Feb 12, 2019
@klingo
Copy link
Contributor Author

klingo commented Feb 12, 2019

Hi, yes this indeed was the originally desired behaviour.

I tried to implement that and managed to block any interaction with the panel itself, but couldn't figure out how to also trigger the collapsing-animation in case the panel is in expanded state. Since it would be bad user experience if the panel interaction is disabled but stays expanded, I removed the first part again.

In the meanwhile (for my own Add-On) I put the checkbox that enables/disables the submenu inside the panel itself. If it is closed you immediately see if the features under that topics are enabled or disabled (due to different text-color); and if you want to change it you anyway open it to check the other settings.
This way I can "hide" the different ON/OFF checkboxes from the main panel and make it look more clean.

So... I'm not sure now which should be the target behavior :)

@sirinsidiator
Copy link
Owner

I'd prefer it to actually disable the interaction with the submenu. That way it is consistent with how the property works on other widgets.

I haven't tried it yet, but calling the animation:PlayFromStart method when the menu is open should be all it takes to close it.

local function UpdateDisabled(control)
    local disable
    if type(control.data.disabled) == "function" then
        disable = control.data.disabled()
    else
        disable = control.data.disabled
    end

    if disable then
        control.label:SetColor(ZO_DEFAULT_DISABLED_COLOR:UnpackRGBA())

        if control.open then
            control.animation:PlayFromStart()
        end
    else
        control.label:SetColor(ZO_DEFAULT_ENABLED_COLOR:UnpackRGBA())
    end
end

@klingo
Copy link
Contributor Author

klingo commented Feb 12, 2019

That's what I was trying as well, unfortuantely it does not work though (i.e. nothing happens).

I made another commit that adds the logic to prevent the submenu from expanding/collapsing when it is set to disabled. So only closing animation should be missing.

@sirinsidiator
Copy link
Owner

Ok. I'll take a look later.

@sirinsidiator
Copy link
Owner

You need to set control.open = false before calling PlayFromStart. Otherwise it will think you just opened the menu in OnStop.
In your last commit you also removed the disabled color. It always shows the label in white now and the color of the arrow also doesn't get adjusted.

@sirinsidiator
Copy link
Owner

@klingo
Copy link
Contributor Author

klingo commented Feb 12, 2019

Oh I see, that's why no animation happened.
Thanks for spotting that; I noticed that I added two local variables for the colors that haven't been used and wanted to delete them; turns out I deleted too much.

In the latest comit I included all your feedback, and from what I can see it is working fine now.

@sirinsidiator
Copy link
Owner

Ok. I'll merge it now. Thanks again for contributing. :)

@sirinsidiator sirinsidiator merged commit 2dd65dd into sirinsidiator:master Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants