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

feat(react-router-scroll): make scroll position storage optional #34530

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jayjayjpg
Copy link

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

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jan 18, 2022
@jayjayjpg
Copy link
Author

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!

@imjoshin
Copy link
Contributor

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/ 😄

@derzierau
Copy link

@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.

@jayjayjpg
Copy link
Author

@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

@derzierau
Copy link

@jayjayjpg thanks for the quick answer. Sure this solution would also be possible but I think having a general approach would be much cleaner.
I'll pick this up in the next weeks.

@jayjayjpg
Copy link
Author

@derzierau That's perfect, thank you for taking a look!

@derzierau
Copy link

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

@TylerBarnes TylerBarnes added topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) topic: plugins Related to plugin system, themes & catch-all for plugins that don't have a label and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Aug 19, 2022
@imjoshin
Copy link
Contributor

imjoshin commented Aug 25, 2022

@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.

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:

  1. Not personal data
  2. Not sent to any backend server (Gatsby or otherwise)
  3. Stored in SessionStorage, so once a browser is closed the information is gone

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!

@derzierau
Copy link

@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.

@imjoshin
Copy link
Contributor

@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. 😄
https://www.gatsbyjs.com/docs/how-to/plugins-and-themes/configuring-usage-with-plugin-options/

I would recommend a config key like disableScrollPositionCache that defaults to false.

@greatwitenorth
Copy link
Contributor

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.

@richard-underwood
Copy link

@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.

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:

  1. Not personal data
  2. Not sent to any backend server (Gatsby or otherwise)
  3. Stored in SessionStorage, so once a browser is closed the information is gone

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!

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.

https://ico.org.uk/for-organisations/direct-marketing-and-privacy-and-electronic-communications/guide-to-pecr/guidance-on-the-use-of-cookies-and-similar-technologies/what-are-cookies-and-similar-technologies/

Cookies that expire at the end of a browser session (normally when a user exits their browser) are called ‘session cookies’. Cookies that can be stored for longer are called ‘persistent cookies’. PECR applies to both types.

PECR applies to any technology that stores or accesses information on the user’s device. This could include, for example, HTML5 local storage, Local Shared Objects and fingerprinting techniques.

PECR does not prohibit using cookies and similar technologies. However, PECR does require you to tell people about them and give them the choice as to whether or not this information is stored on their devices in this way.

https://ico.org.uk/for-organisations/direct-marketing-and-privacy-and-electronic-communications/guide-to-pecr/what-are-pecr/

PECR are the Privacy and Electronic Communications Regulations. Their full title is The Privacy and Electronic Communications (EC Directive) Regulations 2003.

They are derived from European law. PECR implement European Directive 2002/58/EC, also known as ‘the e-privacy Directive’.

@hcherewaty
Copy link

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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) topic: plugins Related to plugin system, themes & catch-all for plugins that don't have a label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants