Skip to content

Conversation

sandy081
Copy link
Member

@sandy081 sandy081 commented Apr 14, 2021

This PR fixes #118077

@Tyriar In order to support custom hovers for activity bar I have to do following changes to hover service:

  • add y coordinate to target
  • fix the service when x and y coordinates are passed. It seems I am the first one to use these coordinates.

Please review these changes

@sandy081 sandy081 added this to the April 2021 milestone Apr 14, 2021
@sandy081 sandy081 requested a review from Tyriar April 14, 2021 14:10
@sandy081 sandy081 self-assigned this Apr 14, 2021
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Changes look fine, if we are going to do this elsewhere we should probably handle this via options instead, for example being able to specify left/right anchor position, vertical alignment and a gap.

Comment on lines -167 to -168
let targetLeft = this._target.x !== undefined ? this._target.x : Math.min(...targetBounds.map(e => e.left));
if (targetLeft + this._hover.containerDomNode.clientWidth >= document.documentElement.clientWidth) {
Copy link
Member

Choose a reason for hiding this comment

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

FYI this was meant to protect the consumer from displaying the hover in an invalid position.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it and it makes sense when x is being computed. If the value is coming from outside, I would assume its caller responsibility. In my case, this check is breaking the hover when sidebar is on the right. Hence removed this check when coordinates are passed.

@sandy081
Copy link
Member Author

I agree it would be nice if there are options like you mentioned. In my case passing coordinates is good enough for now.

@sandy081 sandy081 merged commit af1b506 into main Apr 14, 2021
@sandy081 sandy081 deleted the sandy081/activitybar/hover branch April 14, 2021 14:31
@github-actions github-actions bot locked and limited conversation to collaborators May 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use custom hover for activity bar tooltip
2 participants