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

Clicking + to add annotation scrolls page back up #4570

Closed
2 tasks done
macobo opened this issue Jun 2, 2021 · 7 comments
Closed
2 tasks done

Clicking + to add annotation scrolls page back up #4570

macobo opened this issue Jun 2, 2021 · 7 comments
Assignees
Labels
bug Something isn't working right good first issue Good for newcomers

Comments

@macobo
Copy link
Contributor

macobo commented Jun 2, 2021

Bug description

Peek.2021-06-02.15-46.mp4

This makes adding annotations especially annoying

Expected behavior

No scrolling

How to reproduce

  1. Go to insights
  2. Try to add insight
  3. See page scroll up

Environment

  • PostHog Cloud
  • self-hosted PostHog, version/commit: latest master

Additional context

Thank you for your bug report – we love squashing them!

@macobo macobo added bug Something isn't working right good first issue Good for newcomers core-experience labels Jun 2, 2021
@samwinslow
Copy link
Contributor

Curious to see how this gets solved... I noticed this behavior affects a few text inputs as well.

@macobo
Copy link
Contributor Author

macobo commented Jun 4, 2021

Probably a stopPropagation for the link click event? :)

@mariusandra
Copy link
Collaborator

mariusandra commented Jun 4, 2021

Did a quick test and removing autoFocus from the textarea fixes it... but that's not an ideal solution. I'll try upgrading antd next.

@mariusandra
Copy link
Collaborator

Nope, no change from updating antd. Not sure what's causing the issue then.

@alexkim205
Copy link
Contributor

Curious to see how this gets solved... I noticed this behavior affects a few text inputs as well.

@samwinslow Hmm interesting, I'm gonna take a stab at this one now, but could you point me to specific text inputs where we see similar issues? Maybe I can batch all the fixes up in one PR if they're related

@alexkim205
Copy link
Contributor

alexkim205 commented Jun 4, 2021

Okay after a bit of digging I think I see what's happening.

Some Context

There are two types of AnnotationMarker's we generate for our graphs: tooltips for existing annotations and modals for creating an annotation. For each marker, we create a full-height, full-width, absolutely positioned div and insert the modal as a child.

// For each annotation marker modal, insert this to the end of body
<div style="position: absolute;top: 0px;left: 0px;width: 100%;/* height: 50px; */">
   <div>
      <div class="ant-tooltip ant-tooltip-placement-bottom  ant-tooltip-hidden" style="left: -895px; top: -958.909px; transform-origin: 50% -4px;">
         <div class="ant-tooltip-content">
         <div class="ant-tooltip-arrow">
         <span class="ant-tooltip-arrow-content"></span>
      </div>
      <div class="ant-tooltip-inner" role="tooltip">PostHog is up-to-date</div>
   </div>
</div>

Root Cause

For the first type of marker (annotations we already know exist), we render them on first mount of the graph since we know where these markers should be positioned. We see them being appended to the end of <body>'s children.

Screen Shot 2021-06-04 at 1 53 13 PM

For the second type of marker (modal to create an annotation), the modal's position depends on which date the user has clicked on to create an annotation.

Today's code dynamically renders an AnnotationMarker every time a user clicks the + sign and opens this marker. Because the marker's boundaries are defined by the full-size viewport (see context), the browser scrolls to the top of the page to display the largest bounding element (<div style="position: absolute">) containing this marker.

Screen Shot 2021-06-04 at 1 56 49 PM

When the user clicks away or closes the modal, the AnnotationMarker is unmounted.

Solution

Render a hidden create annotations marker on first mount of graph component so that it isn't rerendered everytime a user clicks to create a new annotation.

Using getPopupContainer prop and defining modal in parent graph component so that root modal doesn't get rerendered on every hover action.

@mariusandra
Copy link
Collaborator

Closed with #4610

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants