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

feat: Use zustand middleware to persist to local storage #3767

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Oct 4, 2024

This PR is a rethink of two Friday quick fixes -

Originally, I was just resolving a React error and then tidying up some unused store variables.

In this approach, I'm using Zustand's persist middleware pattern (docs) to handle the variable stores in local storage. Here's the benefits to this approach as I see them -

  • Less boilerplate in components that would need local or session storage - we set this up once in a store
  • An easy pattern to follow if we needs something like this elsewhere (e.g. the staging banner, session (!) or notifications when we get there)
  • Using a store means the variable could still be controlled by multiple React components
  • Less complex styled components (passing props, remembering to use shouldForwardProp etc)
    • I've also used a MUI Collapse component to save passing these variables into the styled component

Copy link

github-actions bot commented Oct 4, 2024

Removed vultr server and associated DNS entries

Comment on lines +155 to +162
<Collapse
in={showSidebar}
orientation="horizontal"
collapsedSize={SIDEBAR_WIDTH_MINIMISED}
sx={{ height: "100%" }}
easing={"ease-in-out"}
timeout={200}
>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff here is a bit weird - I've just wrapped this component in a MUI Collapse (docs) instead of controlling this via variables passed to a styled component.

@DafyddLlyr DafyddLlyr requested a review from a team October 7, 2024 07:32
@DafyddLlyr DafyddLlyr marked this pull request as ready for review October 7, 2024 07:34
Copy link
Contributor

@RODO94 RODO94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the Application tab and all seemed to go well for me with the Store object in there.

{"state":{"showSidebar":true},"version":0}

Only checked info I seen in this PR in terms of code changes, nothing outwith what I seen here

]);

const [activeTab, setActiveTab] = useState<SidebarTabs>("PreviewBrowser");
const [isSidebarMinimised, setIsSidebarMinimised] = useState<boolean>(() => {
const savedState = localStorage.getItem("isSidebarMinimised");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DafyddLlyr is the gain that we swap this for the code above it:

  const [resetPreview, isFlowPublished, toggleSidebar, showSidebar] = useStore((state) => [
    state.resetPreview,
    state.isFlowPublished,
    state.toggleSidebar,
    state.showSidebar,
  ]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we also remove the togglePreview() function below as it's now handled by Zustand.

hideTestEnvBanner: () => set({ isTestEnvBannerVisible: false }),
}),
{
name: "editorUIStore",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Local Storage the default here or have I missed where you specified this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, local storage is just default so we don't need to specify anything here.

@DafyddLlyr DafyddLlyr merged commit 5963fd5 into main Oct 7, 2024
12 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/zustand-persist-middleware branch October 7, 2024 15:46
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