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

Ec issue #29 bug fix - Lists in the Nav Bar throw error when local storage is empty #60

Merged
merged 19 commits into from
Apr 4, 2024

Conversation

3campos
Copy link
Collaborator

@3campos 3campos commented Apr 3, 2024

Description

The list elements in the Navigation Bar contained a const declaration that used the "getItem" method to obtain a value from local storage. When this method was run, an error would occur when local storage was empty or did not contain a value for the key, "tcl-shopping-list-path". Specifically, the application would throw an error and the home page would be blank after signing in to the app.

Updates

To resolve this bug that stemmed from the NavigationBarSingleList component, I changed the localStorageListName const declaration to an arrow function and added a try-catch statement.

Before Bug Fix

image

Testing Steps / QA Criteria

Steps to reproduce the behavior:

  • Git pull
  • Open the branch named ec-issue-29-bug-fix
  • Run npm start
  • Sign out of the application by using the Sign Out button in the lower-left hand corner.
  • Open the Inspect Element tool on Chrome and open the "Application tab". Confirm that the local storage is empty/cleared.
  • Sign in to the application. The home page should appear. Confirm that no error is thrown by the application in the console log.

eonflower and others added 17 commits February 1, 2024 18:11
Copy link

github-actions bot commented Apr 3, 2024

Visit the preview URL for this PR (updated for commit 31ee93d):

https://tcl-66-smart-shopping-list--pr60-ec-issue-29-bug-fix-9cho9qzz.web.app

(expires Thu, 11 Apr 2024 23:47:12 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2dc16cb2a79cbd6723fdc511b73e0743df1d18d0

Copy link
Collaborator

@eonflower eonflower left a comment

Choose a reason for hiding this comment

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

Nice job Emilio. Everything seems to be working on my end!

Copy link
Collaborator

@jeremiahfallin jeremiahfallin left a comment

Choose a reason for hiding this comment

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

This does fix the issue of the page breaking when the user has never been on a list and they sign in for the first time but it loses the feature of the bullet point being blue for the list the current user is on.

You should be able to fix the one issue while keeping the blue bullet feature.

If you need a hint: Try console.logging localStorageListName, and then look into optional chaining: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

@3campos
Copy link
Collaborator Author

3campos commented Apr 4, 2024

This does fix the issue of the page breaking when the user has never been on a list and they sign in for the first time but it loses the feature of the bullet point being blue for the list the current user is on.

You should be able to fix the one issue while keeping the blue bullet feature.

If you need a hint: Try console.logging localStorageListName, and then look into optional chaining: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

Thanks, @jeremiahfallin. I fixed the issue using your optional chaining solution. I also fixed a related problem with a blue background not being applied to the nav bar list element that is being viewed on the list page.

@3campos 3campos requested a review from jeremiahfallin April 4, 2024 05:40
Copy link
Collaborator

@jeremiahfallin jeremiahfallin left a comment

Choose a reason for hiding this comment

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

Nice fix, approved!

@3campos 3campos merged commit 8078eb5 into main Apr 4, 2024
2 checks passed
@3campos 3campos deleted the ec-issue-29-bug-fix branch April 4, 2024 23:48
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.

3 participants