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

Guide user to add their first tiem #24

Merged
merged 9 commits into from
Mar 1, 2024
Merged

Conversation

ocsiddisco
Copy link
Collaborator

Description

On the List Page, the user sees now a different interface displayed, depending of their experience with the application:
-> After creating their very first list, the user will see a message, a gif showing how to add an item, and a redirection button to the manage-list.
-> When a user creates a new list (and already has at least one), they will see a nice message and a redirection button to the manage-list.
-> When a user already has items in its list, it will display the items and the search component.

On Sign Out, the local storage is now being emptied.

Related Issue

Closes [#8]

Acceptance Criteria

[x] The list view, when there are no items to display, should show a prompt (e.g., a button) for the user to add their first item

Type of Changes

Enhancement & bug fix

Updates

Before

Screenshot 2024-02-27 154157

After

when a user is creating their first list

Screenshot 2024-02-27 154542

when a user has created a new list (and already has at least one list)

Screenshot 2024-02-27 154138

when a user has items present in their list

Screenshot 2024-02-27 154157

Testing Steps / QA Criteria

  • git pull origin ce--ju--add-first-item
  • npm start

List Page:

  • Select a list and check whether the message displayed is adequate.
  • Test with an existing user (once with empty list, once with items) and a newly created user.

Local Storage:

  • On Logout, check your local storage, it should be empty.

Copy link

github-actions bot commented Feb 27, 2024

Visit the preview URL for this PR (updated for commit 385b67b):

https://tcl-71-smart-shopping-list--pr24-ce-ju-add-first-it-g5aguim3.web.app

(expires Thu, 07 Mar 2024 15:37:26 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 1e7ade9d0f374c4ddb5d7ab6fc541062fc7a1ab4

Copy link
Collaborator

@vivitt vivitt left a comment

Choose a reason for hiding this comment

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

Thank you for the work you have done and for fixing the logout bug!

I have a concern regarding the GIF that was included. I think the information it provides could be replaced by a brief text asking users to create their first list (something like 'Start by creating a list...'), as you are doing with the list items. I believe this will offer a clearer explanation and avoid any unneeded motion that might cause discomfort to some users.

If you prefer to keep the GIF, I think we can improve the alt text to make it more useful for users who rely on it. Also, I noticed that there's a warning related to the GIF: 'Assets in the public directory cannot be imported from JavaScript...'
Maybe it would be nice to fix it so we don't have to remind later to do so.

Finally, something that is out of the scope of the issue but could be nice to think about… I wonder if it wouldn't be a good idea to display the list and manageList links only when users have lists to navigate or manage. I think maybe this could result in a more intuitive user experience... What do you think?

Thank you again for your work. I look forward to hearing your thoughts.

@BikeMouse
Copy link
Collaborator

Hey Viviana,

Well the task merely said to add a prompt to help users to add a first item to an empty list, but I totally see what you mean - after all a new user need to create a list first before being able to add an item.

We had wanted some visual support as to show the user what to do to add an item, but I wonder if a screenshot might be better - no motion, but still the benefits on that visual aid. For clarity we could add some text above to explain what needs to be done.

Copy link
Collaborator

@borjaMarti borjaMarti left a comment

Choose a reason for hiding this comment

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

Hey team, well done!!

Tested the functionality both with no lists and already having lists and it works as expected.

I agree with Viviana about only showing the List and Manage-List links when the user already has lists (especially because the List.jsx view doesn't have a case for data.length === 0 && lists.length === 0, resulting in a null return), but that's out of scope for this issue.

The only thing I noticed: are we still using AddItem.gif? The reference to it was removed, but the file itself is part of the commit. Should we delete it?

Other than that, looks good! 🥳

@BikeMouse
Copy link
Collaborator

Truth to be told I hesitated to remove the file, but will do that right now

@BikeMouse BikeMouse requested a review from Amaka202 February 29, 2024 15:36
Copy link
Collaborator

@Amaka202 Amaka202 left a comment

Choose a reason for hiding this comment

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

Well done team! This works as expected 💥💥

@BikeMouse BikeMouse merged commit cbea07d into main Mar 1, 2024
2 checks passed
borjaMarti added a commit that referenced this pull request Mar 2, 2024
Merged PR #25 with main after fixing merge conflict with PR #24.
@vivitt vivitt deleted the ce--ju--add-first-item branch March 7, 2024 08:44
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.

5 participants