Ej/background maps settings flag #737
Conversation
lightlii
requested review from
gmaclennan and
ErikSin
and removed request for
gmaclennan
There was a problem hiding this comment.
Good job! I left some minor comments/suggestions
2 things:
- Once you rebase/merge from master, I would like to give it one more quick look before you merge.
- I added alot of maps in testing. When it is loading the list of maps, it blocks the UI. I think that is fine when it is initially loading, but Tan stack query makes the API call everytime the window is out of focus and refocused, causing the UI to be blocked. I think we should disable invalidating the query. And when the users uploads a new map, we can use the
refetchfunction to refresh the list. That way the list is not constantly trying to refetch even when we know there is no new data.
There was a problem hiding this comment.
I believe the SettingsList is only being used by the Experiments Menu. Is that the case, I think we need to rename it as "ExperimentsList" or something more general like "listItem". Right now it implies it is being used for the settings menu not the experiments menu.
|
Hey I think I've added what you requested in (2) in your comment above. So it's ready to look at again I'd say! |
|
Oops sorry, nope I missed error handling in |
This reverts commit 108cf02.
lightlii
force-pushed
the
ej/background-maps-settings-flag
branch
from
dec835d
to
9fd494e
Compare
|
@ErikSin I've rebased from master, so this is ready for a last look over |
|
Im unable to open it. Are you able to open it on your local device? |
I don't have any issues running it. Lets look at that together when we call later |
|
There are some import errors that the linter and vs code are catching |
…/mapeo-desktop into ej/background-maps-settings-flag
Adds 'Experiments' list to settings menu with
Background Mapsas (currently only) option.When Background Maps flag is activated, Background maps becomes an available option in settings.
closes #733
Preview