Skip to content

persist data source branch in localStorage - #89

Merged
jamesdabbs merged 3 commits into
mainfrom
jcd/store-branch
Dec 9, 2023
Merged

jamesdabbs merged 3 commits into
mainfrom
jcd/store-branch

Conversation

@jamesdabbs

@jamesdabbs jamesdabbs commented Dec 9, 2023 •

Copy link
Copy Markdown
Member

Testing Notes

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

@jamesdabbs jamesdabbs self-assigned this Dec 9, 2023
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Dec 9, 2023 •

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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

jamesdabbs commented Dec 9, 2023 •

Copy link
Copy Markdown
Member Author

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
@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 <steven.clontz@gmail.com>
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