Skip to content
This repository has been archived by the owner on Jun 14, 2020. It is now read-only.

Calling qtip("disable") toggles status rather than disabling due to no state variable #597

Closed
roblouie opened this issue Sep 12, 2013 · 4 comments

Comments

@roblouie
Copy link

I'm using qTip2 for validation error messages. Because I have dynamic validation, I end up calling disable multiple times on my qtips. I was at first mystified why this was actually toggling them between enabled and disabled.

A little digging revealed that if you call qtip("disable"), it calls the disable function with no 'state' argument. 'state' comes back undefined, so the status gets toggled.

I was able to get around it with $("something").qtip("api").disable(true), rather than simply calling qtip("disable"), but I believe this should be fixed.

@roblouie
Copy link
Author

I decided to get a bit more clever and just do this in the qtip js file, seems to be working:

PROTOTYPE.toggleActive = function(state) {
    if(this.destroyed) { return this; }

    if('boolean' !== typeof state) {
        state = !(this.rendered ? this.tooltip.hasClass(CLASS_DISABLED) : this.disabled);
    }

    if(this.rendered) {
        this.tooltip.toggleClass(CLASS_DISABLED, state)
            .attr('aria-disabled', state);
    }

    this.disabled = !!state;

    return this;
}

;PROTOTYPE.disable = function() { return this.toggleActive(TRUE); };

PROTOTYPE.enable = function() { return this.toggleActive(FALSE); };

@Craga89
Copy link
Contributor

Craga89 commented Sep 13, 2013

disable actually expects a state to it, i.e. .qtip('disable', true) to disable :) Might be mildly confusing, but it follows the same pattern for toggle, so I'll be keeping it this way. Sorry to see you were confused by this!

@Craga89 Craga89 closed this as completed Sep 13, 2013
@Craga89 Craga89 reopened this Oct 31, 2013
@Craga89
Copy link
Contributor

Craga89 commented Oct 31, 2013

For the purposes of transparency (woo open-source) here's a conversation about this bug we had, where I'll play the part of both Rob and Myself ;)


This isn't strictly true (from my perspective, it's your code but hear me out!). Toggle certainly does take a state, but the name 'toggle' implies that. You are toggling on and off. You then use 'hide' and 'show', which call toggle with the state.

Disable does not mean toggle, in the same way 'hide' does not mean toggle. That would be like having hide expect a state and having no show method. That's why locally in your code I made:

PROTOTYPE.toggleActive = function(state) {
 //Code that was in disable here.
}

PROTOTYPE.disable = function() { return this.toggleActive(TRUE); };
PROTOTYPE.enable = function() { return this.toggleActive(FALSE); };

To me that is more intuitive.

@Craga89
Copy link
Contributor

Craga89 commented Oct 31, 2013

You make a valid point actually Rob, I was a bit quick to brush you off so apologies for that!

What are your thoughts on .qtip('disable', 'toggle'), where .qtip('disable') disables but the former toggles? Similar signature to jQuery's .animate('toggle') stuff.

EDIT: I'm doing that, as it sounds reasonable and keeps within the jQuery signatures :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants