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

Convert all demos to Stackblitz to prevent the auto-scrolling glitch #1415

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

sequba
Copy link
Contributor

@sequba sequba commented Jun 18, 2024

Context

After analyzing the issue, I found that it's caused by Codesandbox, which, when embedded in an iframe, calls Element.scrollIntoView() or some similar API method to cause the entire page to scroll to the demo.

In my opinion, the best solution is to convert all demos to Stackblitz, which doesn't do that and is, moreover, generally more reliable. @evanSe approved that solution.

Alternative solutions:

  1. Temporary position the demo on top of the page and then bring it back to its original position after a delay:
<iframe onload="console.log('absolute'); this.style.position='absolute'; this.style.top='-9999em'; setTimeout(() => { console.log('static'); this.style.position='static'; }, 800);">

Work less reliably, delay must be pretty long, if a user scrolls to the demo before the timeout they won't see the iframe.

  1. Delay initialization of an iframe until a user scrolls to its vicinity.
<iframe loading="lazy">

Auto-scrolling still happens, but only when the user scrolls the guide to a section close to the demo. For short guides (e.g., https://hyperformula.handsontable.com/guide/integration-with-vue.html), lazy-loading will work the same as the default loading.

How did you test your changes?

Tested locally. Verified that all demos in the guides:

  • work correctly in Stackblitz
  • are displayed correctly in iframes in the guides
  • don't cause the auto-scrolling glitch

Types of changes

  • Breaking change (a fix or a feature because of which an existing functionality doesn't work as expected anymore)
  • New feature or improvement (a non-breaking change that adds functionality)
  • Bug fix (a non-breaking change that fixes an issue)
  • Additional language file, or a change to an existing language file (translations)
  • Change to the documentation

Related issues:

  1. Fixes [Bug]: Docs scrolls automatically to #demo on refresh #1412

Checklist:

  • I have reviewed the guidelines about Contributing to HyperFormula and I confirm that my code follows the code style of this project.
  • I have signed the Contributor License Agreement.
  • My change is compliant with the OpenDocument standard.
  • My change is compatible with Microsoft Excel.
  • My change is compatible with Google Sheets.
  • I described my changes in the CHANGELOG.md file.
  • My changes require a documentation update.
  • My changes require a migration guide.

@sequba sequba self-assigned this Jun 18, 2024
@sequba sequba linked an issue Jun 18, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Jun 18, 2024

Performance comparison of head (4762263) vs base (b18f616)

                                     testName |   base |   head | change
------------------------------------------------------------------------
                                      Sheet A |    531 | 534.97 | +0.75%
                                      Sheet B | 173.27 | 172.48 | -0.46%
                                      Sheet T | 151.14 | 150.26 | -0.58%
                                Column ranges | 553.14 | 560.64 | +1.36%
Sheet A:  change value, add/remove row/column |  14.55 |  14.15 | -2.75%
 Sheet B: change value, add/remove row/column | 141.69 | 132.16 | -6.73%
                   Column ranges - add column | 156.92 | 154.51 | -1.54%
                Column ranges - without batch | 458.31 | 465.38 | +1.54%
                        Column ranges - batch |  123.7 | 122.36 | -1.08%

@sequba sequba changed the title Convert all demos to Stackblitz Convert all demos to Stackblitz to prevent the auto-scrolling glitch Jun 18, 2024
@evanSe
Copy link
Member

evanSe commented Jun 18, 2024

Could you check if this resolves the issue eg vertical-scroll 'none' <iframe src="..." allow="vertical-scroll 'none'"></iframe>

@evanSe
Copy link
Member

evanSe commented Jun 19, 2024

@magierg could you verify everything is good from a docs perspective, thanks!

@sequba
Copy link
Contributor Author

sequba commented Jun 19, 2024

Could you check if this resolves the issue eg vertical-scroll 'none' <iframe src="..." allow="vertical-scroll 'none'"></iframe>

I checked this. It doesn't work

@sequba sequba requested a review from evanSe June 19, 2024 10:39
@sequba
Copy link
Contributor Author

sequba commented Jun 19, 2024

@evanSe @magierg it's ready for CR and testing.

@sequba sequba merged commit 7f3a5b1 into develop Jun 20, 2024
17 of 21 checks passed
@sequba sequba deleted the feature/issue-1412 branch June 20, 2024 09:24
sequba added a commit that referenced this pull request Jun 20, 2024
* Convert angular demo to stackblitz to fix no-automatic-preview codesandbox issue (#1405)

* Convert all demos to Stackblitz to prevent the auto-scrolling glitch (#1415)

* Convert basic operations demo to Stackblitz

* Convert all the demos in the guides to Stackblitz

* Set iframe height to 590px for all stackblitz demos
@magierg
Copy link
Contributor

magierg commented Jun 26, 2024

this has been verified both locally - all the demos work, but most of them (those build on parcel) have long loading time and throw console errors - fixing it will require updating these demos to newer version of parcel or (even better) to vite - this has been discussed with @evanSe as well

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.

[Bug]: Docs scrolls automatically to #demo on refresh
3 participants