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

Theme previewer: Update back button to navigate all the way back to the theme details page. #114

Closed
wants to merge 2 commits into from

Conversation

ryelle
Copy link
Collaborator

@ryelle ryelle commented Jun 5, 2024

Fixes #87. In that issue, it was suggested that the variations and patterns not add history entries. Unfortunately, the navigation of the iframe causes the entries, so we can't get around that. Instead, I've tried a different method. Now, the code tracks how many variations & patterns you've selected, and then uses that count to go backwards X times so that you're back at the theme details page.

If the previewer was opened in a new window, it realizes the history is too short, and just falls back to a regular link to the details view.

Currently, this does not handle navigating inside the iframe, for example if you click a post link, it will throw off the history counter. If this approach sounds good I can look into fixing this.

Screenshots

history.mp4

How to test the changes in this Pull Request:

  1. View a theme archive
  2. Open the previewer (in the same tab)
  3. Click around variations and patterns
  4. Click the previewer back button, it should take you to the theme details page
  5. Click your browser's back button, it should take you back to the theme archive you came from

@fcoveram
Copy link

fcoveram commented Jun 7, 2024

  1. Click your browser's back button, it should take you back to the theme archive you came from

If I understand correctly, and based on the video attached, this step takes you to the archive page "Author: WordPress.org" where 4 themes are list, right? If so, is it possible to go back to the theme details page as in step 4?

From the quote below I understood that it is possible. But I could have got it wrong.

…Now, the code tracks how many variations & patterns you've selected, and then uses that count to go backwards X times so that you're back at the theme details page.

@ryelle
Copy link
Collaborator Author

ryelle commented Jun 7, 2024

If I understand correctly, and based on the video attached, this step takes you to the archive page "Author: WordPress.org" where 4 themes are list, right? If so, is it possible to go back to the theme details page as in step 4?

Correct, step 5 goes back to the page before the theme details, in this case, the author archive. You can think about the history like a stack of cards, we're just navigating back and forward through them. Steps 1-3 create the stack (I missed one step, 1.5, click a theme to view the detail page), and then steps 4 and 5 navigate back through them. In step 4, we jump all the way back to the details page. Then in step 5 we go back once more, to get to the author archive. From here, you could use the browser forward button to "get back" to the detail view, moving up the stack. Or you could click a different theme, restarting the stack.

history

@fcoveram
Copy link

fcoveram commented Jun 7, 2024

Very clear. I like how it works and don't envision confusing flows ✨ 🚀

@ndiego
Copy link
Member

ndiego commented Jun 7, 2024

This flow looks great 🤩

@StevenDufresne
Copy link
Contributor

I can get the state out of sync using the forward button.

Test case:

  • Start on: http://localhost:8888/zeever/ in an Incognito window
  • Open "Preview"
  • Click on 2 patterns.
  • Click interface "Back" button
  • Click Browser forward button
  • Select same patterns (I don't think it matters)
  • Click the interface "Back" button
    • Notice you are reversing through patterns.
  • Click the interface "Back" button
    • Notice the button doesn't work anymore

Video: https://d.pr/i/qwuOfq

@ryelle
Copy link
Collaborator Author

ryelle commented Jun 10, 2024

I can get the state out of sync using the forward button.

😩 yeah… I noticed that later too, but I can't figure out a way to detect "forward" navigation. Do you have any suggestions?

@ryelle ryelle force-pushed the try/back-through-state branch from 3adbade to e111117 Compare June 10, 2024 18:57
@ryelle
Copy link
Collaborator Author

ryelle commented Jun 10, 2024

Caught back up on my work from Friday and I think I actually did figure this one out— the change in e111117 should work even if the user clicks browser back/forward, because it tracks the difference in total history entries. IIRC, I didn't push it on Friday because I found another edge case, but I've just tested it again and it seems to work. @StevenDufresne could you give this update a try?

@ndiego
Copy link
Member

ndiego commented Jun 10, 2024

@ryelle is it possible to test the changes on the live site, or only in a testing environment?

@ryelle
Copy link
Collaborator Author

ryelle commented Jun 10, 2024

Not without merging it, and at this point I'd rather not merge until we know it's workable.

@ndiego
Copy link
Member

ndiego commented Jun 10, 2024

Not without merging it, and at this point I'd rather not merge until we know it's workable.

Yeah, sounds good.

@StevenDufresne
Copy link
Contributor

This isn't working for me, it appears like in Chrome replaceState creates an entry. I can confirm that it does work in FireFox.

@ryelle
Copy link
Collaborator Author

ryelle commented Jun 11, 2024

This isn't working for me, it appears like in Chrome replaceState creates an entry. I can confirm that it does work in FireFox.

replaceState shouldn't create an entry, but the iframe navigation event does, and tracking history.length accounts for that.
I'm not sure what bug you ran into. Either way, I'm able to get into a weird spot by clicking the browser back button, then clicking the app back button — it pulls me back one too many pages (on Chrome and FF).

It sounds like this is just a limit of the current history API — you can't read the history entries, or determine where in the stack you are. In the (maybe near) future we should get a cross-browser Navigation API which should help, but it's not implemented in FF or Safari yet.

Given that, I'm inclined to close this PR and leave the current behavior (a normal link) for the Back button. Maybe we can revisit this in a couple months if the Navigation API becomes available, or maybe updating the previewer will also change how we do this navigation.

@ryelle ryelle closed this Jun 12, 2024
@ryelle ryelle deleted the try/back-through-state branch June 12, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hard to navigate back after you have previewed a theme.
4 participants