-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
feat(react-router-scroll): make scroll position storage optional #34530
base: master
Are you sure you want to change the base?
feat(react-router-scroll): make scroll position storage optional #34530
Conversation
I still need some more info on how to pass Gatsby plugin options to package source files before this is ready for review. I'll also check out the contribution docs in the meantime! |
Thanks for contributing! If you're curious, the docs for plugin configuration can be found here: https://www.gatsbyjs.com/docs/how-to/plugins-and-themes/configuring-usage-with-plugin-options/ 😄 |
@jayjayjpg hey there, can someone give me an update about the situation here? Actually, all important sites of publishers (Welt, Bild f.e.) in Europe can only use Gatsby anymore if we don't add this feature to the config. Furthermore, I'll offer you to finish the PR if you are interested in. |
@derzierau If you'd like to pick this up and/or pair on it, that'd be awesome! Eventually I ended up just installing a Gatsby fork in the project I needed the changes in, but having this configuration option available for the entire community would be super helpful |
@jayjayjpg thanks for the quick answer. Sure this solution would also be possible but I think having a general approach would be much cleaner. |
@derzierau That's perfect, thank you for taking a look! |
actually, the problem here is, that the ScrollContext for scroll restoration is part of the root element which wraps every generated site (see: packages/gatsby/cache-dir/root.js) therefore it is currently not possible to use the api runner methods to wrap the root component with the scroll context. Also, there is the dependency between location and scroll context which is hard to unbind. Maybe a way would be to use the page data object to add a property to the scroll context. But I dont think that is the appropriate way to change settings for actual plugins. @jayjayjpg |
Hey @derzierau, could you provide some examples of this violating GDPR or other laws? While this does store information on the browser, this information is:
We believe this does not violate any privacy policies, but are open to discussing further if you have concerns or can point to specific rules that this violates. Thank you for your concern! |
@j0sh77 our privacy department alarmed us that storing not functional related content inside local or session storage is a user device access which is only permitted if the functionality of the site depends on it. For us (Axel Springer) it could be a risk and therefore we are aiming to remove it. Don't get me wrong it is not high risk but for a company our size, every little detail could be. There should be the option to opt out of the behavior. |
Ahhh ok. I personally would argue that it is functional content - the way a user expects a site to function depends on this data being stored. That said, if this PR was to add the config property and had tests added, I would accept it. 😄 I would recommend a config key like |
Just wondering if this will be implemented? We are using Termly, and now we have hundreds of uncategorized cookies. To add to the issue, Termly doesn't support bulk categorization of cookies or wildcard matching, so we are stuck having to check for new cookies every time a new page is published. |
The requirement comes from PECR - although there's no personal data, users must be informed of all cookies and similar technologies, which includes session storage. While I'd personally think this'd be fine to cover with a sweeping statement in the cookie policy, anyone using automated tools will have a problem with these, particularly as they each have a unique name. The UK's ICO provides the guidance here, I've copied the relevant paragraphs below.
|
Wondering the same and I agree. While session cookies may not require consent per GDPR, companies are expected to provide information on what these cookies do and why they are necessary. We're implementing our cookie policy via Termly as well and just uncovered hundreds of uncategorized cookies related to this issue. Termly also automatically displays the number of cookies per category in a user's cookie preference center which isn't ideal IMO with so many session cookies that aren't strictly necessary. |
Description
With this change, it should be possible for Gatsby end users to disable the storage of the user's scroll position by the
gatsby/react-router-scroll
package.This is a Work in Progress and still requires another update to allow the enabling/disabling of the scroll position's storage via plugin options.
Documentation
Related Issues
Addresses #27308