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

issue-20[BUG-FIX] #47

Merged
merged 1 commit into from
Mar 31, 2024
Merged

issue-20[BUG-FIX] #47

merged 1 commit into from
Mar 31, 2024

Conversation

StefieCaff
Copy link
Collaborator

Description

I had originally built an if statement in App.jsx. It checked on the loading state which was being set in useShoppingListData. If the loading state was true, the app would render a spinner component to let the user know something was happening. This was however dependent on data. When data was not already saved in the browser (as in during development) this caused a permanent loading state. The deployed app, or any new browser were constantly loading because there was no data to trigger the loading state to change.This was the first thing App.jsx did so it could not access the the Login/ Sign Up logic to fetch the data.

  • The if statement was removed from App.jsx
  • Loading state management and a <Spinner/> we added to the SignInButton and SignOutButton logic in useAuth.jsx.

Related Issue

closes #46

Acceptance Criteria

None

Type of Changes

bug fix

Updates

Before

This is the deployed App

issue-20.BUG-FIX.mov

This is the dev environment in an new browser window
https://github.com/the-collab-lab/tcl-66-smart-shopping-list/assets/108297341/8171c9c0-1c82-4af8-91fd-4f9dbabf87f9

After

This is the dev environment in an new browser window
issue-20{BUG_FIX -after
r window

Testing Steps / QA Criteria

  1. Open a new browser window that you do not use for development
  2. Copy and paste this into the input https://tcl-66-smart-shopping-list.web.app/) -- the loader should render
  3. Navigate to the main branch, copy and paste the dev url (http://localhost:3000/) into the input -- the loader should render
  4. Navigate to sc-issue-20[BUG-FIX] branch, copy the dev url (http://localhost:3000/) into the input -- the loader should NOT render

remove loading state from app that was dependent on data.
add loading state to SignInButton and SignOutButton in useAuth.jsx.
Copy link

Visit the preview URL for this PR (updated for commit 873d641):

https://tcl-66-smart-shopping-list--pr47-sc-issue-20-bug-ey9wgoul.web.app

(expires Sat, 06 Apr 2024 03:29:08 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2dc16cb2a79cbd6723fdc511b73e0743df1d18d0

Copy link
Member

@deeheber deeheber left a comment

Choose a reason for hiding this comment

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

Tested this on my machine in an incognito window on Firefox + it worked on my end. Thanks for looking into this. 🚀

I saw an error in the console for the PR preview version saying I needed to add the domain to the authorized domains in the firebase console. After doing that...it worked.

This feels like an overall issue that collab lab should work out as the PR preview URLs will be random as it seems kinda wild to have to do this every time you wanna test a PR preview URL as this console doesn't allow for wildcard domains.

@StefieCaff
Copy link
Collaborator Author

Tested this on my machine in an incognito window on Firefox + it worked on my end. Thanks for looking into this. 🚀

I saw an error in the console for the PR preview version saying I needed to add the domain to the authorized domains in the firebase console. After doing that...it worked.

This feels like an overall issue that collab lab should work out as the PR preview URLs will be random as it seems kinda wild to have to do this every time you wanna test a PR preview URL as this console doesn't allow for wildcard domains.

@deeheber Thank you for checking this out and taking that extra step in Firebase. Is there anything I can do to follow up on that?

@StefieCaff StefieCaff merged commit 9ed0956 into main Mar 31, 2024
2 checks passed
@StefieCaff StefieCaff deleted the sc-issue-20-BUG] branch March 31, 2024 01:15
@deeheber
Copy link
Member

@StefieCaff no I'll give feedback to the curriculum team about this. I think it's a minor hiccup with the new addition of firebase auth to the project. Probably something in the automation setting it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

20. [BUG} Constant login state on deployed app
2 participants