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

Replace bootstrap tooltips with css tooltips from Primer #79

Merged
merged 1 commit into from
Feb 22, 2016
Merged

Replace bootstrap tooltips with css tooltips from Primer #79

merged 1 commit into from
Feb 22, 2016

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Feb 20, 2016

Fixes #67. These tooltips are purely css, no javascript required.

Before:
chrome_2016-02-20_16-32-56

After:
after

@AlMcKinlay AlMcKinlay self-assigned this Feb 21, 2016
@AlMcKinlay
Copy link
Member

Really like this, they look much nicer, and they are very responsive.

👍 from me :-)

@AlMcKinlay AlMcKinlay added Type: Feature Tickets that describe a desired feature or PRs that add them to the project. second review needed labels Feb 21, 2016
@astorije
Copy link
Member

Hey @xPaw, very nice change!

I noticed 3 little things that could be improved before I give my +1:

  • The previous tooltip had smaller top and bottom paddings, it felt less bulky. Could you change that?
  • Similarly, the previous tooltip was either not as white, or a bit transparent. Would be great to have that here too.
  • Finally, there was a nice transition on appear/disappear that has been removed. It was a nice integration with the rest of the UI.

I know you already posted a before/after but here is mine without zoom, to better feel the difference:

Before After
screen shot 2016-02-21 at 14 23 51 screen shot 2016-02-21 at 14 24 17

Thanks! :-)

@xPaw
Copy link
Member Author

xPaw commented Feb 21, 2016

The previous tooltip had smaller top and bottom paddings, it felt less bulky. Could you change that

That's line-height I think, will change.

Finally, there was a nice transition on appear/disappear that has been removed. It was a nice integration with the rest of the UI.

transition should work.

Similarly, the previous tooltip was either not as white, or a bit transparent. Would be great to have that here too.

I actually like the full white better.

@astorije
Copy link
Member

That's line-height I think, will change.
transition should work.

Great!

I actually like the full white better.

Fair enough, I'll see how it looks with the previous 2 changes :-)

@xPaw
Copy link
Member Author

xPaw commented Feb 21, 2016

Done.

@astorije
Copy link
Member

Sweet! 👍 and merging.

astorije added a commit that referenced this pull request Feb 22, 2016
Replace bootstrap tooltips with css tooltips from Primer
@astorije astorije merged commit dd86522 into thelounge:master Feb 22, 2016
@astorije astorije assigned AlMcKinlay and unassigned AlMcKinlay Feb 25, 2016
@xPaw xPaw deleted the tooltips branch March 7, 2016 17:17
@astorije astorije added this to the 1.2.0 milestone Apr 1, 2017
@AlMcKinlay AlMcKinlay removed their assignment Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants