3DStreet / 3dstreet

🚲🚢🚌 Web-based 3D visualization of streets using A-Frame
https://3dstreet.app
Other
260 stars 33 forks source link

"New" button for all users & global state management discussion #791

Closed rahulkgupta closed 1 month ago

rahulkgupta commented 2 months ago
kfarr commented 1 month ago

I made new button available to all users here: https://github.com/3DStreet/3dstreet/pull/823

I looked at triggering add layer panel but it was more difficult than expected because the state of whether or not the add layer panel is open is in another component (main component?) and not the toolbar component. Maybe revisit after redux?

vincentfretin commented 1 month ago

Yes you have a state isAddLayerPanelOpen in the Main component. The New button is indeed far away from Main component, it's Main>SceneGraph>ToolbarWrapper>Toolbar. You can define a callback const setAddLayerPanelOpen = (open) => { this.setState({isAddLayerPanelOpen: open}); } that you pass as prop in SceneGraph, then in ToolbarWrapper then Toolbar, that's not ideal for maintenance and is called prop drilling. What is better is creating a context, and use the context in Toolbar. This is what we did for Geo or Auth context. https://react.dev/learn/passing-data-deeply-with-context

It's been years since I used redux (that was in 2015), I had very bad experience with how we used it in a project, too much boilerplate, creating an action for every field in a form, rendering perf issues. This was before https://redux-toolkit.js.org/ @reduxjs/toolkit that simplifies the redux api so you don't write so much boilerplate anymore apparently. You still use react-redux package for integration with react. Note that React now have a good context api with useContext https://react.dev/reference/react/useContext and useReducer https://react.dev/reference/react/useReducer that was not the case in 2015, so you may not have a need for an additional library like redux if you don't need the store outside react.

About the trends, I see the react community on X and YouTube uses solutions like react-query, zustand or jotai for their global state management. zustand https://zustand-demo.pmnd.rs/ is similar to redux in the concepts, it started to be above @reduxjs/toolkit in the usage. https://npmtrends.com/@reduxjs/toolkit-vs-jotai-vs-mobx-vs-react-redux-vs-redux-vs-zustand

I personally used jotai https://jotai.org in a project, it integrates well with React's context and hooks, that's a bit similar to Solid signals. For some states like the microphone muted state because I need to access it from react and also in some event listener outside of react. In react code, it's a bit like a context and useState, not prop drilling.

Here is an example:

in atom.js

import { atom, createStore } from "jotai";
export const jotaiStore = createStore();

export const micEnabledAtom = atom(true);

export const getMicEnabled = (): boolean => {
  return jotaiStore.get(micEnabledAtom);
};

export const setMicEnabled = (enabled: boolean) => {
  jotaiStore.set(micEnabledAtom, enabled);
};

in index.js

import { Provider as JotaiProvider } from "jotai";
<JotaiProvider store={jotaiStore}>
  <App />
</JotaiProvider>

in MicButton.js

export const MicButton = () => {
  const [micEnabled, setMicEnabled] = useAtom(micEnabledAtom);
  ...
}

If you don't need the setter, simply:

const isBotEnabled = useAtomValue(isBotEnabledAtom);

in some callback:

cameraRig.setAttribute("player-info", {
  muted: !getMicEnabled(),
  name: name,
});
vincentfretin commented 1 month ago

Also note that we are using in this project for example Events.emit('opengeomodal'); https://github.com/3DStreet/3dstreet/blob/afb7aa2862a27ff427854be412d16c74800a0a02/src/editor/components/components/GeoPanel/GeoPanel.component.jsx#L12-L23 as a way to access the state setter, registering a listener just for that https://github.com/3DStreet/3dstreet/blob/afb7aa2862a27ff427854be412d16c74800a0a02/src/editor/components/Main.js#L139-L142

What would be more familiar to a react developer is defining a setter

setGeoModalOpened = (open) => {
  posthog.capture('geo_modal_opened'); 
  this.setState({isGeoModalOpened: true});
}

and pass setGeoModalOpened as prop to GeoModal component.

With jotai, that could be rewritten like this:

in GeoModal.js:

const setGeoModalOpened = useSetAtom(geoModalOpenedAtom)
const onClick = () => {
  setOpenGeoModal(true);
};

in Main.js:

const isGeoModalOpened = useAtomValue(geoModalOpenedAtom)

in atom.js:

export const geoModalOpenedAtom = atom(false)
rahulkgupta commented 1 month ago

im open to using zustand, it seems like it's a popular library with decent integrations. Regardless of choice, global state management would be really good. So, if we're in agreement w/ zustand, let's go ahead and start adding it into the project!

kfarr commented 1 month ago

closed by #823

new tickets based on discussion:

vincentfretin commented 1 month ago

I'm okay with zustand with the simplest form. I just want to avoid the reducers and actions, I have PTSD with those. :'D All the open state for modals and panels are good candidates to include in the zustand store, this avoids lots of boilerplate of creating contexts.