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

Fix/tooltip under alignment #1452

Merged
merged 9 commits into from
Nov 7, 2017

Conversation

xtine
Copy link
Contributor

@xtine xtine commented Nov 6, 2017

This fix establishes override properties for the calendar detail tooltips so they are properly aligned on month-at-a-glance view. I also restored the top tooltip arrows so they show up now!

screen shot 2017-11-06 at 10 55 47 pm

screen shot 2017-11-06 at 10 56 10 pm

screen shot 2017-11-06 at 10 55 40 pm

https://github.com/18F/fec-cms/issues/1274

@jenniferthibault
Copy link
Contributor

@xtine in the second screenshot, we seem to have lost the pointy tip of the tooltip altogether, and in the first, it's in a reaaaaaly weird place inside the container.

Here's an example of the shape from design files, though I if my memory is right, I think the point might have been adjusting it's location to what it was pointing at...can't remember.

screen shot 2017-11-06 at 7 49 16 pm

Is this something we could address in this pass? I'm concerned that having the point inside the calendar tooltip looks unexpected and broken.

@xtine
Copy link
Contributor Author

xtine commented Nov 7, 2017

@jenniferthibault: wow, okay I didn't realize that arrow was wrong (just an interesting design 😆) because I've never worked with the calendar tooltip before. Can you post what it should look like in here?

And yeah some time even before merge the tooltip point was lost. I can definitely work on getting that back. I assume the point should just be below the (i) icon in the filter label?

@jenniferthibault
Copy link
Contributor

My memory isn't total garbage! It did used to move!

18F/fec-style#474

I'm trying to hunt down what the design is for the tooltip in the calendar grid view, because the overlapping close X and the event title are off too. Hold plz while I dig some old issues up.

@jenniferthibault
Copy link
Contributor

@xtine via https://github.com/18F/FEC/issues/561 I found an old image of what the grid view tooltip is supposed to look like:

screen shot 2017-11-06 at 8 22 43 pm

You're right about the point—it should be directly centered under the thing that initiates it opening, which is either the circular information icon, or the calendar event

@jenniferthibault
Copy link
Contributor

@codecov-io
Copy link

codecov-io commented Nov 7, 2017

Codecov Report

Merging #1452 into release/public-20171109 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@                   Coverage Diff                   @@
##           release/public-20171109   #1452   +/-   ##
=======================================================
  Coverage                     79.4%   79.4%           
=======================================================
  Files                           46      46           
  Lines                         3297    3297           
  Branches                       497     497           
=======================================================
  Hits                          2618    2618           
  Misses                         679     679

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f6fc20...8442d64. Read the comment docs.

@xtine
Copy link
Contributor Author

xtine commented Nov 7, 2017

@jenniferthibault: thanks for digging up the old PRs and calendar detail design, I found out that the override properties for calendar detail tooltip weren't showing up so I added specificity to match the designs.
Also looking at the stylesheets I noticed that the old tooltip svgs were missing from the latest fec-style so I restored those as well.

@xtine
Copy link
Contributor Author

xtine commented Nov 7, 2017

Also I've made sure that Saturday and Sunday event detail tooltips won't be hidden:

screen shot 2017-11-06 at 11 11 51 pm

screen shot 2017-11-06 at 11 12 00 pm

Copy link
Contributor

@jenniferthibault jenniferthibault left a comment

Choose a reason for hiding this comment

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

THEY'RE SO BEAUTIFUL 😻

Thanks so much for patching these up, everything I'm seeing in your screenshots above looks right 👍

I'm still learning what thing best workflow is for our new pattern library environment, so would love your guidance. Since the tooltips in the pattern library are busted too, would we want to file a partner PR to fix that in the pattern library repo as part of the documentation step in a a PR like this? Or would it be a separate, non-dependent task entirely?

@xtine
Copy link
Contributor Author

xtine commented Nov 7, 2017

@jenniferthibault: I think we should make a non-dependent task. We need to migrate pattern library to circle and I still need to make a style asset migration script so those need to be done first of course. If those weren't issues I would be up for a documentation step but we are a bit behind already.

@jenniferthibault
Copy link
Contributor

Sounds good, I just made that a separate, non-dependent issue in the pattern library for fixing the busted tooltips based on the work that happened in this PR 👍

https://github.com/18F/fec-pattern-library/issues/20

@patphongs patphongs merged commit eb1e1c9 into release/public-20171109 Nov 7, 2017
@LindsayYoung LindsayYoung deleted the fix/tooltip-under-alignment branch February 2, 2018 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants