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

persist data source branch in localStorage #89

Merged
merged 3 commits into from
Dec 9, 2023
Merged

Conversation

jamesdabbs
Copy link
Member

@jamesdabbs jamesdabbs commented Dec 9, 2023

Testing Notes

  • Change branches
  • Refresh the page
  • Should still be on the same data branch

@jamesdabbs jamesdabbs self-assigned this Dec 9, 2023
Copy link

cloudflare-workers-and-pages bot commented Dec 9, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d24f73b
Status: ✅  Deploy successful!
Preview URL: https://7b7462ae.topology.pages.dev
Branch Preview URL: https://jcd-store-branch.topology.pages.dev

View logs

@jamesdabbs
Copy link
Member Author

It's easy enough to add basic support for a ?branch= param, but I'm a little unsure about some of the sharp edges. I'm assuming that loading a page with ?branch=... should load the data from that branch and persist it (including the source) into localStorage, right? If so, that means that subsequently leaving the site and returning will leave the user on a different branch without them explicitly having switched branches ... are we sure we want that sort of UX?

@StevenClontz
Copy link
Member

Is it possible to remember intent here? If you follow a query string to a branch, it's forgotten on the next visit without a query string, but if you choose via dev to change branches it persists?

@jamesdabbs jamesdabbs marked this pull request as ready for review December 9, 2023 04:08
@jamesdabbs
Copy link
Member Author

jamesdabbs commented Dec 9, 2023

We can, but I think it's going to be non-trivial. Filed #90 to track that.

Aside: I'm definitely considering anything with a bug label something that needs to be fixed before cut-over. Is there another label we want to use as well? And should it be on #90 , or no? (I'm assuming no?)

which appears to have been introduced through random dependency
drift, not any changes here :/
@jamesdabbs jamesdabbs merged commit 57d30ee into main Dec 9, 2023
3 checks passed
@jamesdabbs jamesdabbs deleted the jcd/store-branch branch December 9, 2023 05:10
jamesdabbs added a commit that referenced this pull request Dec 13, 2023
* format {S#|P#} display (#80)

to include a description of the trait value

* persist data source branch in localStorage (#89)

* persist data source branch in localStorage

* fix compile build failure

which appears to have been introduced through random dependency
drift, not any changes here :/

* cache cypress binary in CI

to fix https://github.com/pi-base/web/actions/runs/7149095438/job/19470902100?pr=89

* fix trait#show page reactivity (#94)

per report [here](https://github.com/orgs/pi-base/discussions/464#discussioncomment-7560692)

* truncate previews at first line break (#96)

* fix filter reactivity (#97)

fixes #77

* add related trait filter url param

* typeset internal links (#99)

* misc. minor style tweaks (#100)

- adjust icon styling
- fix spacing around ,s in alias lists
- correct table header style

* use action verbs in robot toggle (#103)

* Update homepage to promote code4math (#93)

* Update homepage to promote code4math

* typo, formatting

* formatting

* lint

* redirect to normalized ids

---------

Co-authored-by: Steven Clontz <[email protected]>
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