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 # 43 - A user should be able to be directed to a list and view a list name #61

Merged
merged 7 commits into from
Oct 6, 2024

Conversation

eternalmaha
Copy link
Collaborator

@eternalmaha eternalmaha commented Oct 5, 2024

Description

Reopening up a new PR again finally closing out Issue #43 after it was opened again and a reverted PR was merged . Fixed bug issues of duplicate home page rendering and list nav bar still appearing.

In this PR, users should be able to directly navigate to lists after clicking list buttons and list name should appear at the top of list view. This PR also highlighted a new point: list item data will only fetch new lists since our ListItemModel Type was strategically updated in a previously merged PR to include the newly defined parameter: itemQuantity. Incredible team work and insight from the whole team to uncover this new detail!

Related Issue

closes #43

Acceptance Criteria

  • The user is immediately redirected to the list after clicking the name of each list.
  • The name of the list appears at the top of each list view when a user clicks.
  • The user is still able to view each list from the mange-list view.

Type of Changes

enhancement

Updates

list.fetches.data.from.new.lists.mov

Testing Steps / QA Criteria

  1. Sign into application
  2. Select or create a list
  3. List name will appear the top of list view
  4. If list is newly created, list items will appear
  5. Else, list items will not appear if it is an old list

Copy link

github-actions bot commented Oct 5, 2024

Visit the preview URL for this PR (updated for commit 5aa62d6):

https://tcl-77-smart-shopping-list--pr61-ma-view-list-name-ywdjj24f.web.app

(expires Sun, 13 Oct 2024 18:22:12 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: b77df24030dca7d8b6561cb24957d2273c5e9d72

@eternalmaha eternalmaha changed the title Ma/view list name Issue # 43 - A user should be able to be directed to a list and view a list name Oct 5, 2024
@eternalmaha
Copy link
Collaborator Author

eternalmaha commented Oct 5, 2024

Everything appears to be fine so far. Only issue is fetching the data to render in Lists. I realized what the issue was. The list data is passed and fetched properly in Home.tsx and AuthenticatedHome.tsx. The issue was that our fetched items have a different data structure than what our ListItemModel type expects. Our ListItemModel type is expecting the item.quantity property but our fetched items do not have that. When I commented out the itemQuantity defined property in our ListItemModelType, the List component is able to fetch the ListItem data properly…now to work on defining itemQuantity properly in a more appropriate way, which is probably a separate issue

@eternalmaha
Copy link
Collaborator Author

Per the conversations in slack with the team, we have come to the conclusion that the issue does not happen with new lists!!! That makes so much sense now that i think about it. Those old lists had already been created with their defined parameters without the item quantity parameter which is why they lists failed to fetch those old list items. I will go ahead and mark my PR ready for review to then merge my PR. Thank you ever for the perception!

@eternalmaha eternalmaha marked this pull request as ready for review October 5, 2024 23:02
@eternalmaha eternalmaha requested review from bbland1, alex-andria, zahrafalak and RossaMania and removed request for bbland1 October 5, 2024 23:02
@eternalmaha eternalmaha self-assigned this Oct 5, 2024
@eternalmaha eternalmaha added the enhancement New feature or request label Oct 5, 2024
Copy link
Collaborator

@alex-andria alex-andria left a comment

Choose a reason for hiding this comment

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

Great work Maha! I see that the previous rendering half-second flash issue is resolved. This is a helpful feature for your team's app!


// The document's id is not in the data,
// but it is very useful, so we add it to the data ourselves.
item.id = docSnapshot.id;

const decoded = ListItemModel.decode(item);
if (isLeft(decoded)) {
console.error("Validation failed for item:", item);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget to remove all console logs!

@eternalmaha eternalmaha merged commit b1f0487 into main Oct 6, 2024
2 checks passed
@eternalmaha eternalmaha deleted the ma/view-list-name branch October 6, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue # 43. As a user, I want to click on a list and view the name of the list I am viewing when redirected.
3 participants