Skip to content

Remove JS workaround for scroll-padding-top #1596

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

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Jan 16, 2022

Safari didn't previously support it, put it is now supported since version 14 (latest is 15).

Fixes #1334. Related: #1595.

I've tested locally with latest Safari on iPad and iPhone.

Safari didn't previously support it, put it is now supported since
version 14 (latest is 15).
@syphar
Copy link
Member

syphar commented Jan 16, 2022

r? @Nemo157 @GuillaumeGomez

@syphar syphar added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Jan 16, 2022
@syphar
Copy link
Member

syphar commented Jan 16, 2022

@GuillaumeGomez does this approval also include a local test? or is it not needed here?

(to know if I can merge and deploy this)

@GuillaumeGomez
Copy link
Member

I don't have an ipad or an iphone so I can't test locally. I just approved the change in itself.

@syphar
Copy link
Member

syphar commented Jan 16, 2022

Can one of you explain the effect of this change so I can test it?

@jsha
Copy link
Contributor Author

jsha commented Jan 16, 2022

The effect of this change should be nothing- this is a workaround (#873) that's not needed anymore.

If I was incorrect in my evaluation that the workaround was not needed anymore, the effect would be: clicks on internal links (e.g. methods in the sidebar) result in the target item being partially covered by the topbar.

@jsha
Copy link
Contributor Author

jsha commented Jan 16, 2022

Oh and one visible change: the "Uncaught TypeError" described in the title of #1334 should go away, on all browsers.

@GuillaumeGomez
Copy link
Member

Oh and one visible change: the "Uncaught TypeError" described in the title of #1334 should go away, on all browsers.

I was about to fix it. Glad you're removing this code. :)

@syphar syphar added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Jan 21, 2022
@syphar syphar merged commit 556f98f into rust-lang:master Jan 21, 2022
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Jan 21, 2022
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.

maybeFixupViewportPosition: Uncaught TypeError: document.getElementsByClassName(...)[0] is undefined
3 participants