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 swipe to vote change throws you back to the root settings page (#810) #815

Closed
wants to merge 5 commits into from

Conversation

flx-sta
Copy link
Contributor

@flx-sta flx-sta commented Jul 22, 2023

PR Creator Checklist

Ensure you've checked the following before submitting your PR:

  • You've discussed making your changes with a member of the dev team per contributing rules in the README
  • Your changes are free of any lint errors
  • Your changes are free of any typescript errors
  • You've tested your changes

Summary

this fixes #810

  • fix the rerender upon swipeToVote change by buffering the change first
  • add todo notices for missing translations

Screenshots

Simulator.Screen.Recording.-.iPhone.14.-.2023-07-22.at.15.00.49.mp4

Test Plan

  1. Go to Settings -> Appearance
  2. Activate Swipe to Vote
  3. Go to Feed
  4. Try Voting by swiping a post -> should work
  5. Go back to Settings -> Appearance.
  6. Deactivate Swipe to Vote
  7. Go to Feed -> Can't swipe anymore but now you can use the fullscreen "go back" gesture instead

- fix the rerender upon swipeToVote change by buffering the change first
-add todo notices for missing translations
@flx-sta flx-sta changed the title fix #810 - swipe to vote causes rerender swipe to vote change throws you back to the root settings page Jul 22, 2023
@flx-sta flx-sta changed the title swipe to vote change throws you back to the root settings page Fix swipe to vote change throws you back to the root settings page (#810) Jul 22, 2023
Copy link
Collaborator

@sgriff96 sgriff96 left a comment

Choose a reason for hiding this comment

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

This doesn't behave very well since if you leave the settings page before the invisible buffer takes effect, it doesn't apply the settings.

@flx-sta
Copy link
Contributor Author

flx-sta commented Jul 22, 2023

This doesn't behave very well since if you leave the settings page before the invisible buffer takes effect, it doesn't apply the settings.

You mean like when you are "super" fast with the switch and then instantly switch pages?

@sgriff96
Copy link
Collaborator

You mean like when you are "super" fast with the switch and then instantly switch pages?

I didn't even try to be that fast, it was like pretty casual speed imo and still didn't take effect. I can record in a min.

@flx-sta
Copy link
Contributor Author

flx-sta commented Jul 22, 2023

You mean like when you are "super" fast with the switch and then instantly switch pages?

I didn't even try to be that fast, it was like pretty casual speed imo and still didn't take effect. I can record in a min.

I believe you like that, don't worry.
Gotta think of a better solution then :/

If you have an idea feel free to tell me cause the one with leaving the page seemed reasonable

flx-sta added 2 commits July 22, 2023 23:17
# Conflicts:
#	src/components/screens/Settings/Appearance/AppearanceScreen.tsx
@flx-sta
Copy link
Contributor Author

flx-sta commented Jul 22, 2023

It should now get applied properly.

There is still the odd reload when changing its state and then clicking the back button in the header but I have no idea right now how to fix that as that settings affects the whole screen (all of them) due to that global gesture

@gkasdorf
Copy link
Collaborator

It was actually just that the whole stack was getting re-rendered on the change. I adjusted that here in this PR: #823

I copied your changes over for the locale stuff though, since I had forgotten to do that.

@gkasdorf gkasdorf closed this Jul 23, 2023
@flx-sta flx-sta deleted the bug/810 branch July 23, 2023 07:15
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.

[Bug]: swipe to vote switch triggers “back”
3 participants