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: #1713 - jail page keeps reloading #1740

Merged
merged 2 commits into from
Aug 11, 2021

Conversation

niecnasowa
Copy link
Contributor

closes #1713

Frontend checklist

  • Formatted my code with yarn format && yarn lint --fix
  • There are no warnings from yarn lint
  • There are no console warnings when running the app
  • Added any new components to storybook
  • Added tests where relevant
  • All tests pass
  • Clicked around my changes running locally and it works
  • Checked Desktop, Mobile and Tablet screen sizes

@niecnasowa niecnasowa requested a review from a team July 30, 2021 13:32
@lucaslcode
Copy link
Member

@aapeliv can you test this? Need a user to be put in jail

@aapeliv
Copy link
Member

aapeliv commented Jul 31, 2021

Need a user to be put in jail

I put everyone on next in jail.

@aapeliv
Copy link
Member

aapeliv commented Jul 31, 2021

This does seem to fix the problem, but I think we should fix it in some other way than to just hide the whole rest of UI? It just looks very bare?

@aapeliv aapeliv added 0.kind bug report This shouldn't be happening. 1.topic frontend release notes: pending Add to stuff that should be included in release notes labels Jul 31, 2021
@lucaslcode
Copy link
Member

lucaslcode commented Jul 31, 2021

Yeah I guess you're right.
@niecnasowa I thought if a better way - do you want to undo thus change, and instead, to the query in features/useNotifications.ts, you can disable it using the "enabled" option if the user is jailed (useAuthStore().state.jailed) useAuthContext().state.jailed

@darrenvong
Copy link
Member

(useAuthStore().state.jailed)

I think it's better to do it via useAuthContext instead, otherwise we'll end up with two auth stores

@lucaslcode
Copy link
Member

Sorry yes that's what I meant!

@niecnasowa
Copy link
Contributor Author

@lucaslcode Ok, I will change it based on your comments.

@niecnasowa
Copy link
Contributor Author

@aapeliv @darrenvong @lucaslcode Can you recheck it now?

Copy link
Member

@lucaslcode lucaslcode left a comment

Choose a reason for hiding this comment

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

Great, thanks for your contribution!

@lucaslcode
Copy link
Member

You can merge this

@niecnasowa niecnasowa merged commit b9f3098 into develop Aug 11, 2021
@aapeliv aapeliv deleted the frontend/bug/jail-page-keeps-reloading branch August 11, 2021 13:26
@aapeliv aapeliv added release notes: done Was mentioned in release notes and removed release notes: pending Add to stuff that should be included in release notes labels Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind bug report This shouldn't be happening. 1.topic frontend release notes: done Was mentioned in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jail page keeps reloading
4 participants