Fix survol container getting out of height (issue #96)

[rezziemaven]

Changes made:

• js/base.js

  • Noticed that because the innerHTML content doesn't load yet when hovering on a link, the popup height and width are both 0 (this was why the popup would initially appear below the mouse when it should appear above the mouse). I enforced a default value of 320px for each to set an initial popup position so this wouldn't happen.
  • Modified topPosition calculation so that if the mouse is in the bottom half of the screen, then the popup will appear above the mouse, otherwise it will appear below the mouse
  • Set min and max heights for the container (necessary as the popupHeight is initially null and can thus affect placement of the popup above or below the mouse)

• css/base.css

  • Added overflow: hidden rule to truncate popup's contents that may exceed the max height

[mdolr]

• Fixed height doesn't feel right
• If you want the container to appear asynchronously, add it through the templates, as right now, when you stop your cursor on a link, you expect it to show a preview, but even though the content is loaded the container hasn't been updated so it doesn't show anything which feels really weird

[rezziemaven]

Hey @mdolr! Just addressing your two comments:

1. I thought it would be good to enforce a min-height because when I use an initial height value and then in some cases when the content has a height that's actually less than that, it changes the positioning of the popup above/ below the mouse when the mouse moves again, which to me from a user perspective doesn't seem like expected behaviour. But otherwise if the height of the content is actually larger than the min-height it will extend the height of the popup but up to half the height of the entire screen (which I thought would be a good way to ensure that the popup stays within the visible portion of the screen). However, if you'd still like me to remove the fixed height then I'll update it.

2. I agree that this isn't expected behaviour. Would you prefer that it at least shows a blank popup which is then filled with content when it's successfully loaded? I was thinking it could perhaps show some indication that the content was loading in a similar way to how the iPhone previews content in a box when you deep press on a link, and shows a progress bar indicating that the content is loading. However, I know that might need to be a separate issue and outside the scope of this one. Otherwise, yes I can try to handle this in the templates so that the box is the correct size upon hover and mouse move over the link to be previewed.

[mdolr]

I think the problem was more about the preview getting out of the screen when the content is too long and not about the preview being displayed under the cursor and then updated above the cursor (which should also be fixed but is less important honestly).

[rezziemaven]

Hi @mdolr:

Noted. I just made a new commit with the following changes based on your comments:

• Removed the min-height style property set in the last commit
• Removed the fixed/ initial height set in popupWidth and popupHeight in the last commit
• Removed the condition set in the last commit to display none if there's no content to build a preview, removing that weird issue when initially hovering over a link shows no preview
• Set a bottom style property whenever the mouse is in the lower half of the screen, removing the need to know the popup's height
• Removed the popupHeight variable as the height no longer needs to be known when calculating the popup positions

I still left the max-height style property to ensure that the popup stays within the visible part of the screen, to address your requirement that the content doesn't get to be too long. However, the addition of conditionally setting a top or bottom style property ensures that the popup displays above the cursor when in the lower half of the screen, or below the cursor when in the upper part of the screen. This is because it only relies on the cursor height and not the height of the popup which isn't initially known.

Looking forward to your feedback.

[mdolr]

This completely breaks preview for links like :
This one
or
This one

[rezziemaven]

Hi @mdolr,

Apologies for the delay. Could you be more specific as to how it's breaking the preview for stackoverflow links? As in what would you prefer to be the expected behaviour just so I can better understand?

Embedding a GIF to show you what I see when I hover over the links you shared:

Do you mean because I set a max-height for the popup or, with the example of the second one, the last line is clipped at the bottom of the popup?

[mdolr]

Because of the max-height the preview's footer disappears

[rezziemaven]

Thank you for the explanation; apologies for not noticing. I propose the following:

• I can set a fixed height to the main content of the StackOverflow template instead so the footer will remain visible
• I can make it so that the footer is always visible at the bottom of the popup regardless of popup height.

In general though, would you prefer I increase the max-height I set to the popup? My reasoning for making the max-height half of the screen height was so that in that way, the popup would always be visible on the screen.

Regardless, I look forward to hearing your recommendations that would help this PR to be approved.