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

Replace the tab entries on first load #3041

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Replace the tab entries on first load #3041

merged 1 commit into from
Apr 2, 2024

Conversation

jameskerr
Copy link
Member

@jameskerr jameskerr commented Apr 2, 2024

Context
When a tab history object is created, it already contains a single entry with the root pathname "/". In a previous PR, I changed the order of creating session tabs to 1) create the tab, 2) load the url in that tab. This made the code much easier to reason about, but it left this dangling "/" entry in the tab history. The meant the back button was enabled on a newly created session tab. If clicked, the tab would go back to the "/" url which renders a blank page without any buttons. You could not get back to your session if you ended up in this state. Could be very irritating if you needed those session history queries.

Changes
So this PR changes the BrowserTab#load function. It checks if the tab history entries equals ["/"]. If it does, it resets the entries to [] and the index to -1, the pushes the new pathname onto the history. This is exactly what we want. The "action" on the entry is still "push" instead of "replace" so that all the normal things happen.

Tests
There is an e2e test added that checks the back button on a newly created session tab to ensure it is disabled, meaning there is only one item in the tab history entries.

@jameskerr jameskerr merged commit a2e178d into main Apr 2, 2024
3 checks passed
@jameskerr jameskerr deleted the tab-navigation branch April 2, 2024 18:02
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.

2 participants