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.

Sign up for GitHub

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

Jump to bottom

Bring tooltips back #494

Merged
merged 10 commits into from
Sep 8, 2021
Merged

Bring tooltips back #494

merged 10 commits into from
Sep 8, 2021

Conversation

BeritJanssen
Copy link
Member

This branch closes #448 . It sets tooltips based on rdfs.comment, if skos.definition is not available. It also changes the default position of tooltips to is-tooltip-right, since this works better with the layout of the menus. However, text may be cut off (can be scrolled to), so some extra tweaking may be necessary.

Screenshot 2021-07-27 at 21 26 48

Screenshot 2021-07-27 at 21 29 46

@JeltevanBoheemen
Copy link
Contributor

I cannot get the tooltips to behave, they extend the full width of the annotation-edit panel so they require to be horizontally scrolled into view to fully read them. Upon doing that, the panel scrolls back after a short delay. We should either disable that, or make sure the tooltips do not extend past the bounds of the viewport.

@jgonggrijp
Copy link
Member

I intended to finish this branch today, but I'm giving up on it for now, because I want to get a release out. I found this resource on the interaction between positioned elements, z-index and overflow: https://css-tricks.com/popping-hidden-overflow/. In theory, its lessons might apply to our situation, but this remains elusive because there are several positioned elements between the hiding element and the overflowing element.

A CSS-only solution based on a data-tooltip attribute might simply not be flexible enough for our purposes. I think we need something more sophisticated that involves a little bit of JavaScript.

@BeritJanssen
Copy link
Member Author

BeritJanssen commented Sep 1, 2021 •
edited
Loading

The more I think of this, the more I think we should request proper definitions, rather than just comments, from the consortium members. The tooltips should give a short indication of what the category is intended for. The comments provide this information, but also information that an annotator probably wouldn't need, e.g., which classes a given category extends. Perhaps the comments could form the basis of the requested Glossary (#452 ) instead.

@jgonggrijp
Copy link
Member

@BeritJanssen I agree about the tooltip content.

Regarding the technical problem, I just got an idea that I think should work. We could factor out the tooltip part of LabelView to a separate view class. This view consists solely of a transparent <div> with pointer-events: none. The tooltip view comes with a helper function which attaches the tooltip view directly to <body> while also associating it with another view of choice, monkey-patching the latter so that the tooltip view can respond to its mouseenter and mouseleave events. The transparent <div> is layered exactly on top of the associated view in the mouseenter handler.

Would you like to have a go at this? Otherwise, I think I can do this fairly quickly.

@jgonggrijp
Copy link
Member

Should be fixed now. Please review!

  • Tooltip is implemented using a separate, empty, transparent view as I suggested above.
  • attachTooltip helper function creates such a view, appends it directly to document.body and monkey-patches another view (which can be any view) so the tooltip will respond to its hovering events. When the other view is .removed, the tooltip is removed, too, so there is no need to manage it. It even re-binds the event handlers when the other view changes element.
  • Label still includes a tooltip on the right by default, but this is now mediated through attachTooltip.
  • I added some math so the tooltip balloon always stays within the visual viewport (in browsers that support window.visualViewport).
  • In CategoryPicker, the tooltips are now associated with the whole dropdown items instead of only with the labels within them.
  • The Tooltip view is currently still hardcoded to take the skos:definition or rdfs:comment from the class attribute from a FlatItem as the balloon content (which, by the way, is now also language-aware). This could easily be generalized so that it supports arbitrary text extraction from arbitrary models.

@JeltevanBoheemen
Copy link
Contributor

Works! Nice job @jgonggrijp

When hovering a superclass, it obscures the children in the ontology class picker. I think this will mostly be resolved by having shorter class descriptions. Otherwise it may be feasible to move the tooltips on the superclass to 'top' or 'left'.

@jgonggrijp jgonggrijp merged commit b2e62f8 into develop Sep 8, 2021
@jgonggrijp jgonggrijp deleted the feature/tooltip-fix branch September 8, 2021 14:17
@jgonggrijp jgonggrijp added this to the Next release milestone Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JeltevanBoheemen JeltevanBoheemen Awaiting requested review from JeltevanBoheemen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.13.0
Development

Successfully merging this pull request may close these issues.

More tooltips
3 participants
@BeritJanssen @JeltevanBoheemen @jgonggrijp