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

Don't show empty tooltips #1299

Merged
merged 3 commits into from Feb 16, 2015
Merged

Don't show empty tooltips #1299

merged 3 commits into from Feb 16, 2015

Conversation

@muffinmad
Copy link
Contributor

muffinmad commented Jan 15, 2015

Also make tooltip hidden on creation.
Fixing this issue

muffinmad added 2 commits Jan 15, 2015
Also make tooltip hidden on creation
@SergioCrisostomo SergioCrisostomo added this to the 1.5.2 milestone Jan 15, 2015
@SergioCrisostomo
Copy link
Member

SergioCrisostomo commented Jan 15, 2015

@muffinmad nice work!

👍

@@ -170,15 +172,22 @@ this.Tips = new Class({
clearTimeout(this.timer);
this.timer = (function(){
this.container.empty();

var showTip = !this.options.hideEmpty;

This comment has been minimized.

@anutron

anutron Feb 16, 2015 Member

Not a blocker for shipping this, but you declare this variable just to reference it once in a conditional; I'd remove this line and reference the option directly on line 186.

This comment has been minimized.

@anutron

anutron Feb 16, 2015 Member

Ignore this. I see line 183...

this.show(element);
} else {
this.hide(element);
}

This comment has been minimized.

@anutron

anutron Feb 16, 2015 Member

minor style thing; MooTools style is to put these on one line:

if (true) something();
else somethingElse();

only if either of the conditional statements are > 1 line do we add braces (in which case we add braces to both clauses, even if one is one line).

At least, that used to be the convention. Again, not a blocker on shipping it.

arian added a commit that referenced this pull request Feb 16, 2015
Don't show empty tooltips - fixes #1298
@arian arian merged commit 7d556a0 into mootools:master Feb 16, 2015
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.