Persistent query show/hide states

[archiewood]

As a user in dev mode, it's frustrating that query results collapse every time the page reloads.

Desired behaviour:

• The query results are kept as shown / hidden based on what the state was before reload

Rough approach

• Store the state of each query in local storage, and read from there on reload

Questions

• This seems like a weird implementation of storage - I feel like you typically put svelte store variables in store.js?
• But since (I think) you want to control each query independently, you'd perhaps need to independently make variables for each query - and you don't know how many there are before the queries are compiled. So I put them in the component instead.

Next

• It would seem logical to also include the show/hide state of the query text itself, once we've worked out the best approach for this

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
evidence-docs	✅ Ready (Inspect)	Visit Preview	May 12, 2022 at 6:18PM (UTC)

[hughess]

This is great @archiewood. This approach makes sense to me. The one thing I'm going to check is if it would make sense to move into a centralized store as you mentioned.

In our preprocessor, we have access to all the queries before the page is loaded, so we may be able to create all the local storage variables from there and update them in the individual query viewers.

Good point on what we could do next - I think this approach could include:

• show/hide results table
• show/hide written SQL
• show/hide all queries on page

[hughess]

@ud3sh, is there any reason we should avoid using local storage as a solution for persistent settings like these?

[ud3sh]

@ud3sh, is there any reason we should avoid using local storage as a solution for persistent settings like these?

The main thing I can think of is it won't be persisted if they chose to choose a different device or browser. Not sure if that's an issue in this case.

[archiewood]

Yeah @ud3sh I think that's probably desirable behaviour.

If you had two people working on / looking at a project they might want to have different stuff collapsed / expanded

[ud3sh]

Yeah @ud3sh I think that's probably desirable behaviour.

If you had two people working on / looking at a project they might want to have different stuff collapsed / expanded

yeah, that makes sense. What I meant more was if Archie looks at the deployed site on his laptop vs his iPad things would look different. Probably not a big issue in practice.

[hughess]

That makes sense.

@archiewood I think your approach here is a good way to manage that toggle. If we need to centralize the logic later we can think about it, but having the logic for the show/hide toggle in the component it's modifying seems like the right fit.

I'd say you're good to go to add this to the query text show/hide as well!

[archiewood]

Updated to include persistent show/hide for:

• results table
• written SQL
• "master" switch - all queries on page

Question about the master switch: Do we want this to change the state of all queries throughout the project, or just the page the user is on?

Currently this will change throughout the project. Not sure what is best?

[mcrascal]

I like it persisted across pages. "I'm showing my work to someone non-technical, I hide queries, I'm not hitting that button over and over." And vice versa.

[hughess]

I like across pages as well. Should it also do that when the project is deployed?

For the "master" switch, as a user I think I'd want the same behaviour in a deployed site - whereas for the sql query text and results table being open/closed, I think I'd be okay with the page refreshing and the query viewers all being closed again.

[archiewood]

@hughess When you say -"you'd be okay with", do you mean you'd want?

Currently I believe this will not happen. All the queries would remain as they were last on the device.

I can alter the behaviour in deployed (I assume by using the 'dev' flag to determine which mode the site is in)

[archiewood]

@hughess When you say -"you'd be okay with", do you mean you'd want?

Currently I believe this will not happen. All the queries would remain as the last time they were viewed on the device.

I can alter the behaviour in deployed (I assume by using the 'dev' flag to determine which mode the site is in)

[mcrascal]

I think persisting "open" or "closed" between pages makes sense in both dev and prod. Whichever way you go on persistence between pages, I think it'd be preferable for that behaviour to be the same in dev and prod.

[hughess]

I think the way it works now will be great. I don't have a strong opinion about whether the settings should persist in a deployed project, but we can always change the deployed behaviour in the future if we need to.

[archiewood]

Cool - will merge.

[archiewood]

ah so it maybe it should be:

export const showQueries = writable(browser && (localStorage.getItem('showQueries')==='true' || dev ));

since I think dev is 'true'?

want to revert the PR and I'll change this?

I can then add changesets

[hughess]

I'm not sure what the exact logic should be for the localStorage check part of that line, but I think with the most recent proposed line, you could have a situation where:

• The localStorage variable is set to false
• The check in your file will return false since it won't find "true" in localStorage
• (false || true) will resolve to true

What situation does the "||" cover?

[archiewood]

Okay I've tested this locally:

export const showQueries = writable(dev && browser && (localStorage.getItem('showQueries')!='false'));

I think this gives the desired behaviour:

• If you are deployed, queries are hidden by default
• If you are in dev:
  • If showQueries is NOT false (ie true, or not assigned) then queries are shown - the default
  • If showQueries is false, then queries are hidden

[hughess]

That's perfect! 🎉