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

fix: delete map shows confirmation twice #845

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

cimigree
Copy link
Contributor

closes #833

Description

The problem: We were closing the modal immediately after initiating the mutation, but the mutation itself triggers a state change that might cause the modal to re-open. Specifically, the onSuccess handler calls refresh(), which updates the refreshToken and may cause components depending on it to re-render, such as the BackgroundMaps which would then rerender the BottomSheetModal (would still be isOpen true)

Solution: Move the close sheet into the onSuccess handler of removeCustomMapMutation, so the modal closes only after the mutation completes and any state changes settle.

I built an APK to test this because it wasn't happening for me while debugging

To test

You can download map files from here: https://drive.google.com/drive/folders/19iZUWrlTQIBpqr7MnP0u5E5dmFudJYKt?ths=true if you need to
App Settings -> Map Management -> Background Maps -> Choose File
Hit Close once the modal appears
UI Box with Map and Date should be on the screen
Hit Remove Map -> Delete Map
Modal should close and Box with Map should no longer be there
If you go to the map screen it should be back to the default map

@cimigree cimigree requested a review from ErikSin November 14, 2024 18:23
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you replace the < Text/> components in the file?

…ey line up with figma designs. Adds warning red so that the red color is as design specifies
@cimigree
Copy link
Contributor Author

@ErikSin I hope it is ok but while I was doing the text stuff I just thought I would fix it in all of the background map files... Some of what was here didn't match the Figma, so I did everything to match the Figma (if there was a Figma)

@cimigree cimigree requested a review from ErikSin November 18, 2024 17:35
Copy link
Contributor

@ErikSin ErikSin left a comment

Choose a reason for hiding this comment

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

I just noticed one more small thing. The 'Settings/MapManagement/index` uses a < ScrollView > < List /> </ ScrollView > . Can you update that to use the < FullScreenMenuList /> component?

@cimigree cimigree merged commit d14a225 into develop Nov 19, 2024
3 checks passed
@cimigree cimigree deleted the fix/delete-maps-confirmation branch November 19, 2024 15:06
ErikSin pushed a commit that referenced this pull request Nov 25, 2024
* Adds on success handler to the hook so that the modal is only closed after the refresh is done.

* Adds body text and header text to the background map files so that they line up with figma designs. Adds warning red so that the red color is as design specifies

* Uses Full screen menu list for map management menu screen.
ErikSin added a commit that referenced this pull request Dec 17, 2024
* Release v1.1.0

* fix: observations disappearing when tracking starts (#837) (#838)

* fix: Bottom Sheet Opens when animations disabled. (#840) (#841)

* fix: drawer not closing when animations disabled (#842)

* fix: microphone permission modal has correct text (#843)

* Makes sure text in permissions modal is correct. Removes some old console logs.

* fix: delete map shows confirmation twice (#845)

* Adds on success handler to the hook so that the modal is only closed after the refresh is done.

* Adds body text and header text to the background map files so that they line up with figma designs. Adds warning red so that the red color is as design specifies

* Uses Full screen menu list for map management menu screen.

* Fix: share button sometimes not working after first share (#847)

* Fetches a new url for attachments each time the share button is pressed. Uses new BodyText component instead of text.

* chore: remove unused useClearAllPendingInvites hook (#852)

* Removes audio files from sharing an observation. Removes type check that is no longer needed. (#854)

* fix: simplify technical implementation of importing custom map file (#855)

* feat: add number as field (#849)

* feat: add number as field

* chore: remove unnecessary code

* chore: allow negative and decimals

* chore: returned Question Component to original state

* chore: only allow one decimal

* fix: project leaving inconsistencies (#853)

* Uses a state variable and use effect in the bottom sheet modal related to project invitation so the success modal shows properly.

* Fixes crash when permission is taken away. (#851)

* chore: Implement type ramp (#818)

* chore: added rubik and updated text component to use it

* chore: update header

* chore: update description

* chore: update button to use rubik

* chore: update drawer header to rubik

* chore: update text to use variant flag

* chore: update some headers

* chore: revert text and add deprecated tag

* chore: remove all unneccesary fonts

* chore: create header component

* chore: created body text

* chore: update jsdoc comments

* chore:update bodyText to not have line height

* chore: update data and privacy screen to use new text components

* chore:update buttons

* chore: update invite screen

* chore: invite accpeted

* chore: waiting for invite to accept screen

* chore: update drawer text

* chore: text align

* chore: update sync screen

* chore: usefontSize map

* chore: type 24 converted to 14

* chore: update inputs (#830)

* chore: create checkbox using svgs

* chore: update metrics to use new checkbox

* chore: update input color and font

* fix: audio permission modal spacing hiding button (#865) (#873)

* Fixes full sheet on bottom sheet modal back

* Undoes last change. Takes out padding so permission audio fits.

Co-authored-by: cimigree <[email protected]>

* Adds snappoints prop to bottom sheet modal to help sizing on full screen modals. (#878) (#889)

Co-authored-by: cimigree <[email protected]>

* feat: list tracks in observation (#890)

* feat: show linked track in observation (#839)

* chore: create reuseable accordian

* chore: reuseable track

* chore: remove component

* chore: plural of observation

* chore:translations

* chore: create track list

* chore: translations

* chore: fix styling

* chore: update text to use customtext

* chore: rename track list to track accordian

* chore: change nav to popTo from navigate

* chore: remove flex:1

* chore: refactor track list in observation with maintainable architecture (#883)

* chore: find associated track with test

* chore: refactor TrackAccordian

* chore: integrate new tracsk accordian

* chore: pr review

* chore: change to mts file

* chore: remove mock data and testing for ci

---------

Co-authored-by: cimigree <[email protected]>
Co-authored-by: Andrew Chou <[email protected]>
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.

Map / Delete maps shows confirmation x2
2 participants