Use css tooltips on time elements, ability to view time on mobile #824

xPaw · 2016-12-19T14:59:56Z

Fixes #761

astorije

It was slightly annoying on the Send message and Change nick buttons, but on timestamps it really is painful to have the tooltip open as soon as the mouse hovers it, which happens all the time as there is usually a "wall of timestamps" between the chat window and the channel list.

Could we switch to the "delay" version of http://primercss.io/tooltips/ ? And yeah I mean for all buttons. To re-evaluate after trying but I think I'll prefer the consistency.

astorije · 2017-01-04T06:51:06Z

@xPaw, ping?

astorije · 2017-03-31T05:32:12Z

@xPaw, what do you think we should do with this? At the moment it's way too epileptic...

xPaw · 2017-03-31T05:34:09Z

We can experiment with timed tooltips for sure.

This: - Makes tooltips appear after timer instead of instantly, necessary for timestamp tooltips (see #824 (review)) - Uses Primer default animation (not sure if .2s transition was ours or theirs but here it is) - Goes closer to default tooltips which will help to bump future versions and/or to streamline this in build process

astorije · 2017-04-04T05:31:08Z

Hey @xPaw, I have "bumped" our version of Tooltips to the latest one, v0.5.3, which is a 1 stone / 2 birds situation since the timed tooltips are the default ones coming from stock lib.

Only changes I made were the colors + our custom Stylelint config (like :after instead of ::after sadly).

Playing with this on my laptop and mobile, I think it works nicely like that.

Only issue I could not figure out is the following:

(Obviously the first "Change nick" is fake, it was to better compare)
For a reason I could not find, border radius and padding are not respected on the timestamp tooltips. Any idea?

astorije · 2017-04-04T05:35:23Z

client/css/style.css

-	.tooltipped:focus:before,
-	.tooltipped:focus:after {
+	.tooltipped-no-touch:hover:before,
+	.tooltipped-no-touch:hover:after {
 		visibility: hidden;
 		opacity: 0;


Not sure if this needs updating and/or is completely necessary anymore. I just know it works as expected on my mobile at the moment...

Yeah we probably no longer need to disable tooltips on mobile as they are on a delay (?)

Let's keep these at the moment, see how things go at next release or something with the new tooltips, and then we could remove this on mobile. At least for a future PR, this is good to go.

xPaw · 2017-04-07T19:48:04Z

@astorije tooltips are cut (no border radius) because .msg has overflow: hidden. I have removed that attribute and reduced the tooltips patch.

xPaw · 2017-04-07T20:01:10Z

client/css/style.css

@@ -1672,6 +1696,17 @@ kbd {
 	border-right-color: #222;
 }

+@media only screen and (-webkit-min-device-pixel-ratio: 2),


@astorije Do we need this?

This comes from here. I'd keep it as long as it's in the upstream project.

astorije · 2017-04-07T20:24:43Z

reduced the tooltips patch.

Aaaargh, why did you get rid of my commit, @xPaw??? I spent quite some time on it to make sure we wouldn't have to do it again, and made the commit purposedly separate so it made sense to keep it and not squash it and lose history of it.

astorije · 2017-04-07T20:28:57Z

This is the full-size of it, coming directly from the lib. We really really should keep it that way:

commit 7b34216c72d4458e127c41cd2b1055e0958c93f9
Author: Jérémie Astori <jeremie@astori.fr>
Date:   Tue Apr 4 01:27:13 2017 -0400

    Update Primer tooltips to latest v0.5.3

    This:

    - Makes tooltips appear after timer instead of instantly, necessary for timestamp tooltips (see https://github.com/thelounge/lounge/pull/824#pullrequestreview-13676231)
    - Uses Primer default animation (not sure if .2s transition was ours or theirs but here it is)
    - Goes closer to default tooltips which will help to bump future versions and/or to streamline this in build process

─────────────────────────────────────────
modified: client/css/style.css
─────────────────────────────────────────
@ style.css:1516 @ kbd {
 }

 /**
-  * Tooltips
+  * Tooltips v0.5.3
   * See http://primercss.io/tooltips/
   */
+
 .tooltipped {
    position: relative;
 }
@ style.css:1527 @ kbd {
 .tooltipped:after {
    position: absolute;
    z-index: 1000000;
-   display: inline-block;
-   visibility: hidden;
-   opacity: 0;
+   display: none;
    padding: 5px 8px;
    font: 12px Lato;
    line-height: 1.2;
+   -webkit-font-smoothing: subpixel-antialiased;
    color: #fff;
    text-align: center;
    text-decoration: none;
@ style.css:1544 @ kbd {
    content: attr(aria-label);
    background: #222;
    border-radius: 3px;
-   -webkit-font-smoothing: subpixel-antialiased;
-   transition: .2s;
+   opacity: 0;
 }

 .tooltipped:before {
    position: absolute;
    z-index: 1000001;
-   display: inline-block;
-   visibility: hidden;
-   opacity: 0;
+   display: none;
    width: 0;
    height: 0;
    color: #fff;
    pointer-events: none;
    content: "";
    border: 5px solid transparent;
-   transition: .2s;
+   opacity: 0;
+}
+
+@-webkit-keyframes tooltip-appear {
+   from {
+       opacity: 0;
+   }
+
+   to {
+       opacity: 1;
+   }
+}
+
+@keyframes tooltip-appear {
+   from {
+       opacity: 0;
+   }
+
+   to {
+       opacity: 1;
+   }
 }

 .tooltipped:hover:before,
@ style.css:1586 @ kbd {
 .tooltipped:active:after,
 .tooltipped:focus:before,
 .tooltipped:focus:after {
-   visibility: visible;
-   opacity: 1;
+   display: inline-block;
    text-decoration: none;
+   -webkit-animation-name: tooltip-appear;
+   animation-name: tooltip-appear;
+   -webkit-animation-duration: .1s;
+   animation-duration: .1s;
+   -webkit-animation-fill-mode: forwards;
+   animation-fill-mode: forwards;
+   -webkit-animation-timing-function: ease-in;
+   animation-timing-function: ease-in;
+   -webkit-animation-delay: .4s;
+   animation-delay: .4s;
+}
+
+.tooltipped-no-delay:hover:before,
+.tooltipped-no-delay:hover:after,
+.tooltipped-no-delay:active:before,
+.tooltipped-no-delay:active:after,
+.tooltipped-no-delay:focus:before,
+.tooltipped-no-delay:focus:after {
+   opacity: 1;
+   -webkit-animation: none;
+   animation: none;
+}
+
+.tooltipped-multiline:hover:after,
+.tooltipped-multiline:active:after,
+.tooltipped-multiline:focus:after {
+   display: table-cell;
 }

 .tooltipped-s:after,
@ style.css:1711 @ kbd {
    border-right-color: #222;
 }

+.tooltipped-multiline:after {
+   width: -webkit-max-content;
+   width: -moz-max-content;
+   width: max-content;
+   max-width: 250px;
+   word-break: break-word;
+   word-wrap: normal;
+   white-space: pre-line;
+   border-collapse: separate;
+}
+
+.tooltipped-multiline.tooltipped-s:after,
+.tooltipped-multiline.tooltipped-n:after {
+   right: auto;
+   left: 50%;
+   -webkit-transform: translateX(-50%);
+   transform: translateX(-50%);
+}
+
+.tooltipped-multiline.tooltipped-w:after,
+.tooltipped-multiline.tooltipped-e:after {
+   right: 100%;
+}
+
+@media screen and (min-width: 0\0) {
+   .tooltipped-multiline:after {
+       width: 250px;
+   }
+}
+
+.tooltipped-sticky:before,
+.tooltipped-sticky:after {
+   display: inline-block;
+}
+
+.tooltipped-sticky.tooltipped-multiline:after {
+   display: table-cell;
+}
+
+@media only screen and (-webkit-min-device-pixel-ratio: 2),
+   only screen and (min--moz-device-pixel-ratio: 2),
+   only screen and (-moz-min-device-pixel-ratio: 2),
+   only screen and (min-device-pixel-ratio: 2),
+   only screen and (min-resolution: 192dpi),
+   only screen and (min-resolution: 2dppx) {
+   .tooltipped-w:after {
+       margin-right: 4.5px;
+   }
+}
+
 /* End tooltips */

xPaw · 2017-04-07T20:37:12Z

We don't gain anything from including the extra styles, they were removed initially too.

astorije · 2017-04-07T20:40:50Z

We don't gain anything from including the extra styles

Yes we do, as documented in the commit message you got rid of. Benefit is I don't have to spend another hour doing that same 💩 job next time they bump their lib, or investigating why this is not working as expected because we got rid of something we thought was not needed.

Can I re-add my commit, please?

xPaw · 2017-04-08T07:33:03Z

Benefit is I don't have to spend another hour doing that same 💩 job next time they bump their lib, or investigating why this is not working as expected because we got rid of something we thought was not needed.

They haven't updated it since they released it. What I got rid of is multiline and sticky variants because we don't use either of these (and most likely ever won't). I can add the comment about the delay.

This: - Makes tooltips appear after timer instead of instantly, necessary for timestamp tooltips (see #824 (review)) - Uses Primer default animation (not sure if .2s transition was ours or theirs but here it is) - Goes closer to default tooltips which will help to bump future versions and/or to streamline this in build process

astorije

Re-added my split commit for upgrading tooltips lib to v0.5.3 while leaving out the things you removed, @xPaw.
Did one last test and it looks good. Let's do this.

astorije · 2017-04-13T06:00:54Z

client/css/style.css

@@ -1672,6 +1696,17 @@ kbd {
 	border-right-color: #222;
 }

+@media only screen and (-webkit-min-device-pixel-ratio: 2),


This comes from here. I'd keep it as long as it's in the upstream project.

astorije · 2017-04-13T06:01:48Z

client/css/style.css

-	.tooltipped:focus:before,
-	.tooltipped:focus:after {
+	.tooltipped-no-touch:hover:before,
+	.tooltipped-no-touch:hover:after {
 		visibility: hidden;
 		opacity: 0;


Let's keep these at the moment, see how things go at next release or something with the new tooltips, and then we could remove this on mobile. At least for a future PR, this is good to go.

astorije · 2017-04-13T06:06:59Z

I was actually afraid removing the overflow: hidden would make us subject to zalgo text but testing it out, it seems like no.

This: - Makes tooltips appear after timer instead of instantly, necessary for timestamp tooltips (see thelounge#824 (review)) - Uses Primer default animation (not sure if .2s transition was ours or theirs but here it is) - Goes closer to default tooltips which will help to bump future versions and/or to streamline this in build process

Use css tooltips on time elements, ability to view time on mobile

xPaw added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Dec 19, 2016

astorije requested changes Dec 20, 2016

View reviewed changes

astorije mentioned this pull request Dec 21, 2016

Click on the channel's topic to edit it #277

Closed

xPaw added the Meta: Do Not Merge This PR should not be merged. label Dec 29, 2016

astorije assigned xPaw Apr 3, 2017

astorije force-pushed the xpaw/time-tooltip branch from e60107b to 9b485f2 Compare April 4, 2017 04:54

astorije reviewed Apr 4, 2017

View reviewed changes

xPaw force-pushed the xpaw/time-tooltip branch from 7b34216 to b09d1f7 Compare April 7, 2017 19:58

xPaw commented Apr 7, 2017

View reviewed changes

xPaw and others added 3 commits April 13, 2017 01:55

Use css tooltips on time elements

c0a7ae9

Only disable touch tooltips on certain buttons

5d36b29

astorije force-pushed the xpaw/time-tooltip branch from b09d1f7 to 5b721c1 Compare April 13, 2017 05:59

astorije approved these changes Apr 13, 2017

View reviewed changes

astorije removed the Meta: Do Not Merge This PR should not be merged. label Apr 13, 2017

astorije added this to the 2.3.0 milestone Apr 13, 2017

astorije unassigned xPaw Apr 13, 2017

astorije merged commit 5890a3d into master Apr 13, 2017

astorije deleted the xpaw/time-tooltip branch April 13, 2017 06:04

matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017

Merge pull request thelounge#824 from thelounge/xpaw/time-tooltip

6e44fcf

Use css tooltips on time elements, ability to view time on mobile

astorije mentioned this pull request Sep 22, 2017

Toggle user list tooltip shouldn't appear on mobile #1556

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use css tooltips on time elements, ability to view time on mobile #824

Use css tooltips on time elements, ability to view time on mobile #824

xPaw commented Dec 19, 2016

astorije left a comment

astorije commented Jan 4, 2017

astorije commented Mar 31, 2017

xPaw commented Mar 31, 2017

astorije commented Apr 4, 2017

astorije Apr 4, 2017

xPaw Apr 7, 2017

astorije Apr 13, 2017

xPaw commented Apr 7, 2017 •

edited

Loading

xPaw Apr 7, 2017

astorije Apr 13, 2017

astorije commented Apr 7, 2017

astorije commented Apr 7, 2017

xPaw commented Apr 7, 2017

astorije commented Apr 7, 2017

xPaw commented Apr 8, 2017

astorije left a comment

astorije Apr 13, 2017

astorije Apr 13, 2017

astorije commented Apr 13, 2017

Use css tooltips on time elements, ability to view time on mobile #824

Use css tooltips on time elements, ability to view time on mobile #824

Conversation

xPaw commented Dec 19, 2016

astorije left a comment

Choose a reason for hiding this comment

astorije commented Jan 4, 2017

astorije commented Mar 31, 2017

xPaw commented Mar 31, 2017

astorije commented Apr 4, 2017

astorije Apr 4, 2017

Choose a reason for hiding this comment

xPaw Apr 7, 2017

Choose a reason for hiding this comment

astorije Apr 13, 2017

Choose a reason for hiding this comment

xPaw commented Apr 7, 2017 • edited Loading

xPaw Apr 7, 2017

Choose a reason for hiding this comment

astorije Apr 13, 2017

Choose a reason for hiding this comment

astorije commented Apr 7, 2017

astorije commented Apr 7, 2017

xPaw commented Apr 7, 2017

astorije commented Apr 7, 2017

xPaw commented Apr 8, 2017

astorije left a comment

Choose a reason for hiding this comment

astorije Apr 13, 2017

Choose a reason for hiding this comment

astorije Apr 13, 2017

Choose a reason for hiding this comment

astorije commented Apr 13, 2017

xPaw commented Apr 7, 2017 •

edited

Loading