You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "neilramaswamy (via GitHub)" <gi...@apache.org> on 2024/08/29 06:08:30 UTC
[PR] [SPARK-49456][DOCS] Fix hash fragment scrolling behavior on version-specific Spark documentation [spark]
neilramaswamy opened a new pull request, #47925:
URL: https://github.com/apache/spark/pull/47925
### Why are the changes needed?
If you've ever used the Spark docs, you're probably familiar with clicking a link and seeing something like:
<img width="1028" alt="image" src="https://github.com/user-attachments/assets/e2a6a816-da3d-4037-bfda-7a4654397bc6">
The header is half-hidden! How egregious! In some cases, it's even worse—the header can be hidden entirely. You, being the inquisitive person you are, timidly bubble a burning question... "what do you mean by _in some cases, it's even worse_? Are you telling me it's not deterministic?"
Unfortunately, it isn't. In our code base, [we wait 25 milliseconds](https://github.com/apache/spark/blob/d048dfe6d10dfc6367eafd63bb27990d80f460e1/docs/js/main.js#L114) before trying to [manually scroll FIFTY SEVEN pixels](https://github.com/apache/spark/blob/d048dfe6d10dfc6367eafd63bb27990d80f460e1/docs/js/main.js#L94) below the top of the anchor... why 57? I'm not sure. The header is 83 pixels currently, so I'm not sure why we chose 57.
Anyway, the real fix for this is to use `scroll-margin-top`, which allows you to specify a margin that the browser will keep between the top of the element and the top-border of the viewport. To reduce our reliance on magic numbers, I've also refactored the navbar's height into a CSS variable, which now gets used as the argument to `scroll-margin-top`. No more magic numbers!
This feature is [widely supported in all browsers](https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-margin-top#browser_compatibility). But even if a user's browser doesn't support this, their hyperlinks will be off by about ~80 pixels, which is just as bad as the UX is today.
### Does this PR introduce _any_ user-facing change?
Yes. When they click/visit Spark links, the page will _actually_ scroll where they want to go. Crazy right?
### How was this patch tested?
I tested this on Chromium 128.0.6613.85, and Safari 17.4.1.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49456][DOCS] Fix hash fragment scrolling behavior on versioned Spark documentation [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #47925:
URL: https://github.com/apache/spark/pull/47925#issuecomment-2317375656
I have tested PR locally, and it looks good to me.
BTW, can we streamline the PR description a bit? Then, we can keep the git log concise after merging. Thank you @neilramaswamy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49456][DOCS] Fix hash fragment scrolling behavior on versioned Spark documentation [spark]
Posted by "neilramaswamy (via GitHub)" <gi...@apache.org>.
neilramaswamy commented on PR #47925:
URL: https://github.com/apache/spark/pull/47925#issuecomment-2318331677
PR description adjusted. Thanks for the review @yaooqinn.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49456][DOCS] Fix hash fragment scrolling behavior on versioned Spark documentation [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #47925:
URL: https://github.com/apache/spark/pull/47925#issuecomment-2319723053
Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49456][DOCS] Fix hash fragment scrolling behavior on versioned Spark documentation [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #47925: [SPARK-49456][DOCS] Fix hash fragment scrolling behavior on versioned Spark documentation
URL: https://github.com/apache/spark/pull/47925
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org