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

morebits: Remove tipsy dependency, use jQuery UI for tooltips #971

Merged

Conversation

Amorymeltzer
Copy link
Collaborator

This harkens back to the homegrown tooltips that were replaced by tipsy (a14f46f, 2011), mainly because we've still been defining the CSS! Some tweaks were made: with a decade of black, I figured blue would be too bright, so mediumblue was used (we could, of course, keep it black). There should only be minor differences in appearance. The classes are changed, but there's nobody using them (in fact, there are more instances of old morebits pages with the old, pre-tipsy style).

https://api.jqueryui.com/tooltip/

@Amorymeltzer Amorymeltzer added deprecations Functions, etc. from Morebits that are deprecated and should be checked for usage before removal Module: morebits The morebits.js library docs labels Jun 14, 2020
@Amorymeltzer Amorymeltzer force-pushed the morebits-jquerytooltip branch from 8a7d14b to 863557d Compare June 21, 2020 20:35
morebits.css Outdated
.morebits-ui-tooltip
{
padding: 4px 6px 4px 6px;
font-size: 1.2em;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The font size within the tooltip is ENORMOUS at my end - looks like about 18px, not easily measurable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

19.2, yeah. This is annoying skin shit. I was aiming for 12 px, but they behave differently. For a quick example:

  • Vector: 0.8em of 100% = 12.8px
  • Timeless: 1.1em of 0.95em` = 16.72px
  • Modern: 1.1em of x-small = 11px

There's a second issue, though. 12px looks a little small on Vector, because it has font-family: sans-serif whereas 13px is a little big on Modern and Timeless, because they both have font-family: Verdana,Arial,sans-serif.

YMMV, but IMO 13px looks better on Modern and Timeless than 12px does on Vector (and we should be favoring Vector anyway). rem and % are no help AFAICT.

Suggested change
font-size: 1.2em;
font-size: 13px;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default (without any font-size set) looks good to me on vector, monobook and modern. For timeless, it's huge but easily restored back to normal by putting something like

.skin-timeless .morebits-ui-tooltip {
	font-size: 13px;
}

@siddharthvp
Copy link
Member

Also, the tooltip is hiding behind the select2 dropdowns when they're open.

@Amorymeltzer Amorymeltzer force-pushed the morebits-jquerytooltip branch from 863557d to e4ca005 Compare June 28, 2020 15:53
@Amorymeltzer
Copy link
Collaborator Author

Well, we do have .select2-container { z-index: 10000; } everywhere. This was discussed in #692 (comment), although I'm not sure how necessary those are atm. .tipsy was set to 100000, so we were off by a factor of ten.

We could just set the z-index to something similarly asinine (100000 or 10001), or really investigate whether we really still/ever needed the select2 z-index.

@siddharthvp
Copy link
Member

We could just set the z-index to something similarly asinine (100000 or 10001), or really investigate whether we really still/ever needed the select2 z-index.

Yeah, can't trigger any issues after removing the z-index on the select2 dropdowns; feel free to remove that.

@Amorymeltzer Amorymeltzer force-pushed the morebits-jquerytooltip branch 2 times, most recently from 110dd9e to f3bf2f9 Compare June 30, 2020 01:16
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Jun 30, 2020
Per follow-up in wikimedia-gadgets#971 from the addition in wikimedia-gadgets#692, these appear unnecessary.
@Amorymeltzer Amorymeltzer force-pushed the morebits-jquerytooltip branch from f3bf2f9 to 055a6a3 Compare June 30, 2020 18:26
@Amorymeltzer
Copy link
Collaborator Author

There do appear to be a number of scripts that assume jquery.tipsy is available, which is probably a pretty good sign that folks were relying on twinkle/morebits using it. That being said, a quick survey shows:

  • Many are ancient and stale
  • Most are derived from/clones of/inspired by one script, User:Writ Keeper/Scripts/teahouseUtility.js (~100 active users import, ~300 total)
  • The tipsy bit isn't and hasn't been working there anyway

I left a note for WK on the talkpage, but I don't perceive this to be a problem or hindrance to removal.

@Amorymeltzer Amorymeltzer force-pushed the morebits-jquerytooltip branch from 055a6a3 to b319e0e Compare July 2, 2020 10:13
@Amorymeltzer Amorymeltzer added this to the August 2020 update milestone Jul 12, 2020
This harkens back to the homegrown tooltips that were replaced by tipsy (a14f46f, 2011), mainly because we've still been defining the CSS!  Some tweaks were made: with a decade of black, I figured blue would be too bright, so mediumblue was used.  The classes are changed, but there should only be minor differences in appearance.

https://api.jqueryui.com/tooltip/
@Amorymeltzer Amorymeltzer force-pushed the morebits-jquerytooltip branch from b319e0e to cefc232 Compare July 16, 2020 13:46
@Amorymeltzer Amorymeltzer merged commit f4d70da into wikimedia-gadgets:master Jul 19, 2020
@Amorymeltzer Amorymeltzer deleted the morebits-jquerytooltip branch July 19, 2020 11:17
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Sep 1, 2020
Was only used for the tipsy tooltips, removed in cefc232 / wikimedia-gadgets#971; unused elsewhere
wiki-ST47 pushed a commit to wiki-ST47/twinkle that referenced this pull request Sep 2, 2020
Per follow-up in wikimedia-gadgets#971 from the addition in wikimedia-gadgets#692, these appear unnecessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecations Functions, etc. from Morebits that are deprecated and should be checked for usage before removal docs Module: morebits The morebits.js library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants