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

x/website: hovering over links on redesigned website can break scrolling #32739

Closed
dominikh opened this issue Jun 22, 2019 · 11 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dominikh
Copy link
Member

Environment

Firefox 67.0

What did you do?

  1. Go to https://tip.golang.org/doc/
  2. Place mouse cursor over a link, such as Frequently Asked Questions (FAQ)
  3. Try to scroll the page with the scroll wheel. Try scrolling up as well as down, whether the bug reproduces depends on the scroll direction in relation to some state of the hovered link.

What did you expect to see?

I expected the website to scroll.

What did you see instead?

Instead of scrolling the website, I am scrolling… the link.

modern

@dominikh dominikh changed the title website: hovering over links on redesigned website can break scrolling x/website: hovering over links on redesigned website can break scrolling Jun 22, 2019
@gopherbot gopherbot added this to the Unreleased milestone Jun 22, 2019
@agnivade
Copy link
Contributor

@andybons

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 23, 2019
@andybons
Copy link
Member

@dominikh super weird 😕

The extent to which the styling has been changed on that page is text color.

I’m unable to repro using Firefox 67.0.4 (64-bit) on macOS Mojave 10.14.5 (18F132) but I’m using a trackpad to scroll and not a mouse wheel, which may be the issue? It would be very odd if so.

This requires a bit more investigation. I'm assuming you're on linux? This also never occurs on https://golang.org?

@andybons andybons added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 23, 2019
@dominikh
Copy link
Member Author

dominikh commented Jun 23, 2019

The extent to which the styling has been changed on that page is text color.

Changed in comparison to what? https://golang.org/doc/ and https://tip.golang.org/doc/ seem significantly different to me. The diff (ignoring case changes and whitespace) between https://golang.org/lib/godoc/style.css and https://tip.golang.org/lib/godoc/style.css is almost a thousand lines.

As far as reproducing it is concerned: I know several people who cannot reproduce it, but @mvdan can. I am on Linux, I don't know anything about his setup, but I'm sure he'll chime in.

This also never occurs on https://golang.org?

That is correct.

Since I can reproduce the issue reliably, I'll try and see if I can diagnose it further.

@mvdan
Copy link
Member

mvdan commented Jun 23, 2019

I'm on Firefox 67.0.4 on Linux, with a trackpad as well. I get the same behaviour that Dominik gets, as far as I can tell.

I can also reproduce on Chromium 75.0.3770.100, by the way.

@dominikh
Copy link
Member Author

git bisect points at this mega-commit:

commit 4de14c150b85f7b2be471c66202617afea54cdc3 (refs/bisect/bad)
Author: Andrew Bonventre <andybons@golang.org>
Date:   Mon May 20 14:03:19 2019 -0400

    content/static: revamp main header and clean up markup
    
    This is the first of many changes to spiff up golang.org a bit
    to include our updated branding and iterate on the design.
    
    Updates the html to be more modern and rewrites the header CSS to
    be in line with our style guide (golang.org/wiki/CSSStyleGuide).
    
    Additionally, removes the play popup widget in the header nav
    in favor of linking directly to the playground and formats
    the CSS using prettier.
    
    Updates golang/go#9936
    
    Change-Id: I9baf727f9e703b560ef786277b5d9859baa6fbfc
    Reviewed-on: https://go-review.googlesource.com/c/website/+/178137
    Reviewed-by: Katie Hockman <katie@golang.org>

 content/static/go-logo-blue.svg |    1 +
 content/static/godoc.html       |   76 ++++---
 content/static/godocs.js        |   57 +----
 content/static/static.go        |    6 +-
 content/static/style.css        | 1137 +++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------
 5 files changed, 634 insertions(+), 643 deletions(-)

@dominikh
Copy link
Member Author

The h2 and h3 tags have overflow: auto, which explains why they can scroll. As for the sizes of the elements, h3#code (a header I picked arbitrarily) has an inner size of 1610.67x25, while the child a has a total size of 190x27.3333. Why the h3 isn't large enough to contain its a I do not know.

2019-06-24-010832_1808x1163_scrot
2019-06-24-010840_1808x1163_scrot

@dmitshur
Copy link
Contributor

Thanks for doing the investigation and analysis.

Why the h3 isn't large enough to contain its a I do not know.

My guess is that it's related to the fonts available on your system, etc.

It's possible to reproduce on macOS/Chrome by manually forcing a smaller height to the h3 element, e.g., height: 16px;.

I suspect auto isn't the ideal overflow-y value for headings.

@dominikh
Copy link
Member Author

dominikh commented Jun 24, 2019

My guess is that it's related to the fonts available on your system, etc.

Well, yes, that surely is a factor, but not really an explanation for why the h3 isn't automatically sized to contain its children. It doesn't have any size forced upon it by the stylesheet as far as I could see and should thus be as large as its children.

Edit: the tl;dr as to why this happens is that line-height is complex, and CSS is utterly screwed. On a more constructive note, there should be no reason to have overflow: auto on h2 or h3. Removing that will fix the issue.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 24, 2019
@andybons
Copy link
Member

There should be no reason to have overflow: auto on h2 or h3. Removing that will fix the issue.

I agree.

@andybons
Copy link
Member

@gopherbot
Copy link

Change https://golang.org/cl/183597 mentions this issue: content/static: remove overflow: auto for header elements

@dmitshur dmitshur removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 24, 2019
@golang golang locked and limited conversation to collaborators Jun 23, 2020
passionSeven added a commit to passionSeven/website that referenced this issue Oct 18, 2022
A header element should not be forced to scroll when its height
is smaller than its children. Let it overflow instead.

overflow: auto is used to ensure that child floating elements
are contained within their parent. For the case where this is
used (Go version that a function was added), the version number
won’t be any larger than the other text in the header, that
declaration isn’t needed.

Fixes golang/go#32739

Change-Id: I99aea447b19c95840c04af7d7e13280af1ac05de
Reviewed-on: https://go-review.googlesource.com/c/website/+/183597
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants