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

Basic Popper.js re-implementation of asset tooltips #1009

Merged
merged 3 commits into from
May 28, 2019

Conversation

acdha
Copy link
Member

@acdha acdha commented May 23, 2019

This attempts to display the tooltip to the left of the current asset, but Popper.js is smart enough not to fix it there if that would keep it offscreen.

There’s just enough CSS to add a border — this could probably use some additional TLC for more distinct colors and we probably want to adjust widths and font sizes, too, along with possibly using some of the available options to adjust offsets for some of the overlap situations.

See #998

Before

image

After

image
image

This attempts to display the tooltip to the left of the current asset, but Popper.js is smart enough not to fix it there if that would keep it offscreen. 

There’s just enough CSS to add a border — this could probably use some additional TLC for more distinct colors and we probably want to adjust widths and font sizes, too.

See #998
@acdha acdha requested review from klee72 and rstorey May 23, 2019 21:10
@acdha acdha self-assigned this May 23, 2019
@coveralls
Copy link

coveralls commented May 23, 2019

Coverage Status

Coverage remained the same at 68.543% when pulling b41a09e on activity-ui-asset-list-info-tooltip-overhaul into 1070499 on master.

@klee72
Copy link
Collaborator

klee72 commented May 24, 2019

@acdha, is this going to be included in the Tuesday launch? If so, I don't really have the time for refinement work but I can play with the style and options next week some time.

@acdha
Copy link
Member Author

acdha commented May 24, 2019

@klee72 probably not unless we want to put some time into styling it. I don't think it's a show-stopper.

klee72
klee72 previously approved these changes May 24, 2019
acdha added 2 commits May 24, 2019 16:33
This avoids overlapping the thumbnail where possible
Since the tooltip no longer fills the element with the metadata display, which makes it less obvious which asset is being displayed. This adds a hover outline similar to the active asset color to make those states more distinct.
@acdha
Copy link
Member Author

acdha commented May 24, 2019

I made two changes to attempt to position the tooltip under the asset image (it still goes to the sides when the bottom would be outside of the viewport) and to make it a little more distinct which asset is being worked with:

Screenshot 2019-05-24 16 37 14

One question which this raised was whether we still want to preserve the asset-active class when the viewer is closed. It's not immediately obvious why that one asset is orange in the big list but it does make it easier to see the last page you were looking at.

@rstorey rstorey merged commit 8beffc8 into master May 28, 2019
@rstorey rstorey deleted the activity-ui-asset-list-info-tooltip-overhaul branch June 5, 2019 19:59
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.

None yet

4 participants