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

bookmarkable URLs for UI state #202

Open
1 of 3 tasks
scottlamb opened this issue Mar 7, 2022 · 0 comments
Open
1 of 3 tasks

bookmarkable URLs for UI state #202

scottlamb opened this issue Mar 7, 2022 · 0 comments
Labels
bug enhancement javascript Pull requests that update Javascript code

Comments

@scottlamb
Copy link
Owner

scottlamb commented Mar 7, 2022

@krystaDev in #193 started adding bookmarkable URLs for UI state: you can bookmark a live view URL that specifies the layout and cameras. In 782eb2f I extended it so that some of the list view options are bookmarkable.

Three things I'd like to fix/add before calling this feature done:

  1. bug: if you select cameras then switch the layout, the cameras get lost from the URL when it's set here. And the state is remembered in the UI despite not being in the URL anymore which feels weird. Maybe we should be recalculating state from the search params each render so there's no possibility they can get out of sync. I'm trying that for the list view here and it seems to work.
  2. bug: in debug builds, on initial page load (or when not logged in or when there are no cameras), the Javascript console logs No routes matched location "/". I think the problem is simple: the Routes element is in the document here but empty.
  3. missing: in list view, the time range isn't included in the URL, which is a big gap. The problem here is that TimerangeSelector is...intricate. It keeps a bunch of state together in one property that is mutated with this reducer. Some of it goes into the URL (the current selected date(s) and times, the single/multi day selection toggle). Some of it shouldn't (the clickable days, which is derived from state returned from the server). I'm not sure how best to structure the code.
    • Maybe it's time to explore things like redux anyway as we take on more state in the future. I see a stackoverflow question talking about how to use redux with url parameters (but sadly there doesn't seem to be one built-in right way to do it).
    • Or maybe the search parameters should be synced into this state via effect as it's doing now for the allowed days on the given cameras.
    • ...other ideas...?

I wonder if 1 and 2 are symptoms of a pre-existing bad code structure in App.tsx. Part of Live's state, the selected layout, is in App.tsx so that it can be selected in the top menu and used in the main pane. It already smelled a little funny to have part of the activity's implementation in App.tsx. And now with routes this means App.tsx kind of does the routing twice (here and here), which will get worse as we add more activities. I think I can restructure it so that we have a MoonfireFrame component with the overall layout (passing in activityMenuPart and mainComponent). Once logged in, we render a <Routes> to choose the activity with less activity-specific structure in App.tsx. And then each activity can use the frame component and be fully responsible for its own state and menu piece.

scottlamb added a commit that referenced this issue Mar 7, 2022
This structure (described in
#202 (comment))
has less activity-specific logic in App.tsx itself and avoids duplicate
route handling.

This fixes the `No routes matched location "/"` mentioned in #202.
scottlamb added a commit that referenced this issue Mar 7, 2022
I haven't figured out how to expose its state in the URL (for #202),
but documenting how it works today seems like a good first step.
@scottlamb scottlamb added javascript Pull requests that update Javascript code and removed js labels Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enhancement javascript Pull requests that update Javascript code
Projects
None yet
Development

No branches or pull requests

1 participant