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

Es/example of persisted state #747

Merged
merged 3 commits into from
Sep 12, 2023

Conversation

ErikSin
Copy link
Contributor

@ErikSin ErikSin commented Aug 30, 2023

this is incomplete, but I wanted to show an example of encapsulating the logic of whether background maps is on or off directly in the hook.

Also example of doing undefined check inside the useMapStylesQuery hook

@ErikSin ErikSin requested a review from lightlii August 30, 2023 19:34
@ErikSin ErikSin changed the base branch from master to ej/background-map-selection August 30, 2023 19:35
backgroundMapStoreSlice,
'background-maps'
)
export const usePersistedUiStore = createPersistedStore(
persistedUiStoreSlice,
'ui'
)

export const useBackgroundMapStore = () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now this hooks encapsulates the logic for whether background maps is enable or not. And can only show and set the maps that are relevant to that state. No need to the developer to do a check everytime.

: [store.mapStyleLegacy, store.setMapStyleLegacy]
)

return { mapStyle, setMapStyle }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ideally this would return an array, but I am not too sure how to the equivalent of as const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured out how to return an array
image

Comment on lines 45 to 47
data: !legacyStyleQueryResult.data
? [defaultMapStyle]
: legacyStyleQueryResult.data,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this encapsulates the undefined check so the consuming components do not have to do that

Comment on lines 12 to 19
const DEFAULT_MAP = {
id: '487x2pc8ws801avhs5hw58qnxc',
url: 'mapbox://styles/mapbox/outdoors-v10',
bytesStored: 0,
name: m.defaultBackgroundMapName,
isImporting: false,
isDefault: true
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is in 2 places, so ideally should just be moved to one place!

@lightlii
Copy link
Contributor

Hey this makes sense. I've wrapped up the loose ends on this so it can be merged if you're happy with it

@lightlii lightlii marked this pull request as ready for review September 12, 2023 16:34
@lightlii lightlii merged commit c6b63ff into ej/background-map-selection Sep 12, 2023
9 checks passed
@lightlii lightlii deleted the es/exampleOfPersistedState branch September 12, 2023 16:35
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