Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on Aug 5, 2025. It is now read-only.

Implement backward and forward navigation options to iframed window (2)#194

Closed

Conversation

@brandonmcconnell
Copy link
Contributor

Note: This PR builds on the merged (and possibly soon-to-be-revertedPR #181) and removes theset_iframe_scr function andloaded architecture so that the app navigates backward and forward seamlessly.


Overview of changes

This PR adds backward and forward navigation options to iframed window. Because this sort of behavior isn't normally possible within the natural constraints of the History API since the history of the iframed window and that of its containing parent window operate together, I opted to manually track the history of the iframed window in two arrays—history_bwd andhistory_fwd.

Implementation/test cases & detailed steps for each

When any path is manually entered into the iframe's simulated address field…(expand to see details)
  • the currentpath is appended tohistory_bwd
  • history_fwd is reset
  • the newpath is naturally set to the entered path (current functionality)
When the "back" button is clicked…(expand to see details)
  • the currentpath is prepended tohistory_fwd
  • the last item inhistory_bwd is sliced off
  • the sliced item (the previouspath) replaces the currentpath
When the "forward" button is clicked…(expand to see details)
  • the currentpath is appended tohistory_bwd
  • the first item inhistory_fwd is sliced off
  • the sliced item (the nextpath) replaces the currentpath
When any link within the iframe is clicked, it is picked up byhandle_message which then falls back to using thenav_to function for handling and tracking the history accordingly, effectively…(expand to see details)
  • the currentpath is appended tohistory_bwd
  • history_fwd is reset
  • the newpath is naturally set to the entered path (current functionality)

Notes

  • The icons I used are custom ones I threw together in Illustrator, so feel free to use them without any risk of copyright violations, though I understand there are probably plenty of reasons to switch them, even just to stay on brand.
  • I noticed after my initial successful tests that while the back/forward functionality worked as expected, the history persisted across tutorials, so I added an additionalreset_history function that resets the history, which I trigger in thetry clause withinafterNavigate.

Testing

This can be tested/demoed effectively using any of the routing examples:

Screen recording

Screen.Recording.2023-01-23.at.11.51.15.AM.mov

@vercel
Copy link

vercelbot commentedJan 24, 2023

@brandonmcconnell is attempting to deploy a commit to theSvelte Team onVercel.

A member of the Team first needs toauthorize it.

@brandonmcconnellbrandonmcconnell marked this pull request as draftJanuary 24, 2023 21:15
@brandonmcconnell
Copy link
ContributorAuthor

Copying my comments that I added to PR#181 since it was merged:

@brandonmcconnell
Copy link
ContributorAuthor

@Rich-Harris

Sorry, I dropped the ball here, meant to review this PR sooner. I think we should revert it, so I've opened#187. The back/forward buttons in this PR cause the iframe to be reloaded, which gives a very misleading impression of how history navigation works in SvelteKit. It's better to not have history UI than to have UI that causes full page reloads

@Rich-Harris, I think the reload is primarily caused by the iframe being removed and re-added to the screen and how that change interacts with the bwd/fwd history changes.

According to the comment inside theset_iframe_src function…

/**@param {string} path */functionset_iframe_src(src){// removing the iframe from the document allows us to// change the src without adding a history entry, which// would make back/forward traversal very annoyingconstparentNode=/**@type {HTMLElement} */(iframe.parentNode);iframe.classList.remove('loaded');parentNode?.removeChild(iframe);iframe.src=src;parentNode?.appendChild(iframe);}

…removing the iframe altogether and re-injecting it to the page is supposed to "change the src without adding a history entry" though I do not see that actually working in Chrome, even before my PR. Does that work in other browsers? As I navigate around one iframe, I still have to click the browser's back button repeatedly to traverse through every navigation from the iframe.

I can confirm that removing theset_iframe_src function and swapping its usage foriframe.src = resolves this issue, though again, I can't speak to whether any browsers were actually benefitting fromset_iframe_src.

Let's go ahead and revert this PR for now, and either re-open this PR or I can create a new PR that builds on this one and addresses those concerns.

cc@dummdidumm

@brandonmcconnell
Copy link
ContributorAuthor

@Rich-Harris@dummdidumm One idea here which might remove much of the complexity around this is to sync the iframe history/URL state with the browser's so that as the iframe's URL changes, there could be URL param in the browser likeframe_src=${path} so that clicking back in the browser or the iframe essentially trigger the same action and go back one step whether in the iframe or the browser depending on the user's actions. Of course, it would be better to just completely isolate the iframe's history apart from the browser's, but in all my testing and research, there doesn't appear to be a way to achieve that, including the "removing/adding the iframe node" hack.

I'm not sure if/how/where/whenset_iframe_src work(ed), as described in its comment to avoid adding an entry to the browser's history. I'm honestly not criticizing it—it may work in some places—it just hasn't worked in my testing, which granted was limited to Chrome.

@brandonmcconnell
Copy link
ContributorAuthor

I'm converting this back to draft status for the time being. It appears I'll resolve a race condition.

When a user clicks the bwd/fwd buttons repeatedly very quickly, before the first click has a chance to complete its changes to the history, it loads that first click's changes multiple times instead of going bwd/fwd multiple times. I'll likely need to add some sort of a queue so the same changes don't fire twice if the second click is fired before the first click has a chance to make its changes tohistory_bwd,path, andhistory_fwd.

Or simpler yet, I can do a check to see if the previous changes were made yet, and if not, cancel the pending changes and batch the changes in a single execution, so if the user clicks the back button three times very quickly, it would essentially shift three items over (capped to the number of items available ion the history arrays), instead of trying to run each time individually. It could be a lighter load for the iframe this way too.

cc@Rich-Harris@dummdidumm

@vercel
Copy link

vercelbot commentedJan 24, 2023
edited
Loading

The latest updates on your projects. Learn more aboutVercel for Git ↗︎

NameStatusPreviewCommentsUpdated
learn-svelte-dev✅ Ready (Inspect)Visit Preview💬Add your feedbackJan 24, 2023 at 11:52PM (UTC)

@Rich-Harris
Copy link
Member

Thank you for the effort you've put into this, but this isn't the right approach — SvelteKit's whole deal is that you get client-side navigation after the initial render, but settingiframe.src causes an unwanted full page reload. The history traversal needs to happen inside the iframe

@brandonmcconnell
Copy link
ContributorAuthor

brandonmcconnell commentedJan 25, 2023
edited
Loading

@Rich-Harris I didn't add theiframe.src rewriting. Theset_iframe_src function was already doing that, with the added abstraction of removing and re-injecting the iframe node to prevent adding a history entry, which doesn't seem to work anyway, which is why I removed that abstraction.

I can certainly add that function back, but the root issue with the "blinking" from my previous PR,#181, seems to be more linked to the iframe node removal/re-adding than the history traversal.

@brandonmcconnell
Copy link
ContributorAuthor

@Rich-Harris any thoughts on the above? It appears the currently in-use approach still changes theiframe.src directly as you described and pollutes the history.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@brandonmcconnell@Rich-Harris

[8]ページ先頭

©2009-2025 Movatter.jp