-
Notifications
You must be signed in to change notification settings - Fork 242
Implement backward and forward navigation options to iframed window (2) #194
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
Conversation
@brandonmcconnell is attempting to deploy a commit to the Svelte Team on Vercel. A member of the Team first needs to authorize it. |
Copying my comments that I added to PR #181 since it was merged: |
@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 the /** @param {string} path */
function set_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 annoying
const parentNode = /** @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 the 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 |
@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 like I'm not sure if/how/where/when |
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 to 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. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 setting |
@Rich-Harris I didn't add the 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. |
@Rich-Harris any thoughts on the above? It appears the currently in-use approach still changes the |
Note: This PR builds on the merged (and possibly soon-to-be-reverted PR #181) and removes the
set_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)
path
is appended tohistory_bwd
history_fwd
is resetpath
is naturally set to the entered path (current functionality)When the "back" button is clicked… (expand to see details)
path
is prepended tohistory_fwd
history_bwd
is sliced offpath
) replaces the currentpath
When the "forward" button is clicked… (expand to see details)
path
is appended tohistory_bwd
history_fwd
is sliced offpath
) replaces the currentpath
When any link within the iframe is clicked, it is picked up by
handle_message
which then falls back to using thenav_to
function for handling and tracking the history accordingly, effectively… (expand to see details)path
is appended tohistory_bwd
history_fwd
is resetpath
is naturally set to the entered path (current functionality)Notes
reset_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