search

leptos-rs / book

The home for the Leptos book, which can be found deployed at https://book.leptos.dev
MIT License
73 stars 60 forks source link

Pages auto scrolling to code sandbox (only on FF and Linux) #1

Closed paul-hansen closed 3 months ago

paul-hansen commented 9 months ago

The code sandbox causes the page to scroll to it once it loads, this makes reading confusing as you don't notice you aren't starting at the top of the page sometimes. Also makes linking directly to sections not work.

Example: https://leptos-rs.github.io/leptos/view/05_forms.html#controlled-inputs Wait a few seconds after opening that link and once the code sandbox loads it scrolls down to it. Note that link was specifically targeted at the "Controlled Inputs" header which is normally useful for linking people to specific content, but the automatic scrolling to the code sandbox breaks that functionality.

Edit: Only happens for me on Firefox (stable channel v120 64 bit) linux. Does not happen on Chrome on linux and does not happen on Windows. I'm on EndeavorOS (Arch based Linux)

paul-hansen commented 9 months ago

I don't think I noticed this until recently. Possible it was a recent change. Not sure if that's helpful info.

paul-hansen commented 9 months ago

Now that the book's code is public, I tried messing around with it locally using the options from https://codesandbox.io/docs/learn/legacy-sandboxes/embedding but not seeing anything we can do to change this behavior.

I emailed feedback@codesandbox.io about this, I'll report back if I get any solutions.

paul-hansen commented 8 months ago

It seems like it stopped doing this now that I checked again, I'm not sure if it was changed in the Firefox 120.0.1 patch or if Codesandbox was able to find a fix. The last I heard from CodeSandbox was that they were able to replicate the issue and raised it as a bug with their team.

Will close for now and reopen if it happens again or anyone else reports still having the issue.

baldoalessandro commented 5 months ago

Still seeing this in FF 123.0.1 in OSX 🤷‍♂️ ?

paul-hansen commented 5 months ago

Yeah it came back recently, I started noticing it again at least a few days ago.

paul-hansen commented 5 months ago

I confirmed with code sandbox via email that the bug report for this is still open, so they should be aware.

Wonder if we can put these behind a "click to load example" or something to prevent them from moving the page. (maybe only if FF is detected)

paul-hansen commented 5 months ago

Workaround for myself, I've just blocked the iframe (you can use ublock origin or stylus) Since there's still a link to the code example this works pretty well as a workaround.

They started covering half the examples with a popup you have to click to get rid of everytime in iframes anyway so it's still the same amount of clicks since you don't have to dismiss again when opening in a new tab. image

mrchantey commented 5 months ago

Also getting this on windows chrome

anvlkv commented 4 months ago

I'm experiencing this a lot on mac, Firefox.

I think, the sandbox can be opened on demand rather than upfront on page load. I'm kind of person that is going through the docs a lot and it really spoils my experience.

I'd be happy to contribute a PR with this particular improvement if it makes sense to you.

paul-hansen commented 4 months ago

One thing I've been wanting to investigate but haven't had the time is if this is something specific with mdbook.

~~Specifically, I noticed that when setting breakpoints on scroll events it was triggering on this line: https://github.com/rust-lang/mdBook/blob/c671c2e90486b6ec72921a5043dac1c1c5565f76/src/theme/book.js#L654 My theory, (from briefly looking at it) is mdbook is inadvertently catching scroll events happening inside the iframe and trying to apply it's scroll logic to it (maybe setting the scroll position of the wrong document? idk, I don't use iframes much).~~ (Edit: idk that this theory holds up at all. I think you would have to call iframe.contentDockerment.addEventListener so that line shouldn't be triggering for events in the iframe. Could still be something else related to mdbook though)

If someone wanted to confirm this, they could patch and build mdbook removing all code related to scrolling and see if it still happens. If it doesn't, we could create an issue on the mdbook repo and see if we can fix it there.

baldoalessandro commented 4 months ago

Had a look at the script from mdbook you pointed out, and tried to reproduce with the debugger open. I think that the scroll event you see, is the one generated by the browser when the code sandbox iframe get the focus (it became the document's activeElement). I think this is due to this line in the codesanbox code https://github.com/codesandbox/codesandbox-client/blob/aed183fa49072c18adf3d75671ac9116e3b69e4f/packages/app/src/app/pages/Sandbox/Editor/Content/Tabs/index.tsx#L46 (don't be fooled by the false parameter 🙂https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView#aligntotop) which make the editor became visible. I found this on the Chromium issue tracker https://issues.chromium.org/issues/41375023 This is just another theory, I will try to further investigate.

anvlkv commented 4 months ago

While the browser issue seems to have an assignee, and we may hope it will be resolved in observable future w/o my proposed change.

I would argue that showing sandbox on demand (accordion, tab, etc.) would be a more sustainable solution. It wouldn't have to transfer the sandbox code, nor to evaluate it unless the user opens the sandbox. It would also give a little more space to the sandbox iframe making it more convenient to use. Finally a tab or other element can be brought to the top of the page making it more prominent that page offers live examples.

gbj commented 4 months ago

I would argue that showing sandbox on demand (accordion, tab, etc.) would be a more sustainable solution. It wouldn't have to transfer the sandbox code, nor to evaluate it unless the user opens the sandbox. It would also give a little more space to the sandbox iframe making it more convenient to use. Finally a tab or other element can be brought to the top of the page making it more prominent that page offers live examples.

Feel free to make a PR. This sounds fine, it's just not something I have time for at the moment.

anvlkv commented 4 months ago

I've added draft updating one page. Let me know what you think and I can update rest of the pages.

paul-hansen commented 3 months ago

Thanks to @anvlkv this is now behind a collapsed by default section so it doesn't cause these issues anymore. image

If Codesandbox's email support responds with an update I'll try to forward it here but I think we can close this for now since it's no longer causing us issues. Feel free to reopen though if there's still something to do here etc.