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

Reposition popover on scroll & remove tooltip delayed from screen #260

Merged
merged 2 commits into from
Mar 30, 2023

Conversation

danielkaxis
Copy link
Collaborator

@danielkaxis danielkaxis commented Mar 14, 2023

Describe your changes

To give better user experience for phone and pad users, reposition popover on scroll event.
Also remove the tooltip when user, on a touch device, scrolled more than 150 pixels.

Issue ticket number and link

Checklist before requesting a review

  • I have performed a self-review of my own code
  • I have verified that the code builds perfectly fine on my local system
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have verified that my code follows the style already available in the repository
  • I have made corresponding changes to the documentation

Keep popover at anchor element when
user is scrolling
@danielkaxis danielkaxis requested a review from Tigge as a code owner March 14, 2023 15:02
@danielkaxis
Copy link
Collaborator Author

I'm wondering about respositioning the popover on scroll. It has a slight studder when scrolling and I'm not sure if we can fix it with the current implementation of how popover positions itself to the anchor element.

@lekoaf
Copy link
Member

lekoaf commented Mar 15, 2023

Also remove the tooltip when user, on a touch device, scrolled more than 150 pixels.

Do tooltips even show up on a touch device? I thought they only showed on hover...?

@danielkaxis
Copy link
Collaborator Author

Also remove the tooltip when user, on a touch device, scrolled more than 150 pixels.

Do tooltips even show up on a touch device? I thought they only showed on hover...?

Yes, when you touch the element.

@boilund
Copy link
Collaborator

boilund commented Mar 20, 2023

Can you add tests as well?

@danielkaxis
Copy link
Collaborator Author

Can you add tests as well?

Will look into it

Remove tooltip on touch-devices if user
scrolls more than 150 pixels.
@danielkaxis
Copy link
Collaborator Author

Can you add tests as well?

Tests are added! :)

Copy link
Collaborator

@boilund boilund left a comment

Choose a reason for hiding this comment

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

LGTM

@boilund boilund merged commit 06d200f into AxisCommunications:main Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reposition popover on scroll and remove tooltip when scrolling
3 participants