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

added styles using tailwindCss, delete not working #35

Merged
merged 11 commits into from
Mar 31, 2024
Merged

added styles using tailwindCss, delete not working #35

merged 11 commits into from
Mar 31, 2024

Conversation

ijayhub
Copy link
Collaborator

@ijayhub ijayhub commented Mar 24, 2024

Description

I've improved the homepage to give it a more polished and professional look. Now, users can easily highlight and keep track of specific collection IDs (lists) that they want to focus on.

Related Issue

closes #35

Acceptance Criteria

Make everything centered
Remove "Hello from the home (/) page!"
Change "Name of shopping list:" to "Create a shopping list"
Add a button to delete the shopping list
Create a function from the firebase documentation to delete a shopping list/document- not yet
I want the design to look professional.
Highlight the shopping list that is selected

Type of Changes

new feature and UI enhancement .

Updates

Before

before- smart shopping list

After

FireShot Capture 005 - Smart Shopping List - localhost

Testing Steps / QA Criteria

  • Users should be greeted with a visually appealing site upon arrival.
  • Creating a list should result in the ability to highlight any list upon clicking it.
  • Click on a join button if the collection list already exists.

Copy link

github-actions bot commented Mar 24, 2024

Visit the preview URL for this PR (updated for commit 472cb5c):

https://tcl-72-smart-shopping-list--pr35-ij-home-mowl982q.web.app

(expires Sun, 07 Apr 2024 16:24:35 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f7f2b0c98e4c5bce6be0f9b2cd44669f154caa67

@IamHenryOkeke
Copy link
Collaborator

UI Updates looks great and working fine. Great work

Copy link
Collaborator

@IamHenryOkeke IamHenryOkeke left a comment

Choose a reason for hiding this comment

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

UI Updates looks great and working fine. Great work

Copy link
Collaborator

@jmahamed jmahamed left a comment

Choose a reason for hiding this comment

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

I think it looks good, the list can be a bit more centered. I think the background should be just one solid color like #A2A8D3

I think a better title is "Create a shopping list"

button can be "Create new list"

and last button can be "Edit existing list"

Copy link
Collaborator

@redapy redapy left a comment

Choose a reason for hiding this comment

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

Everything is working as expected, great work @ijayhub 🚀 ! I left some non-blocking notes

Comment on lines +236 to +238
await updateDoc(userDocRef, {
sharedLists: arrayRemove(docRef),
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nicely done 💥✨! we don't want the user to have a shared list that does not exist 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay changed that

src/components/SingleList.jsx Show resolved Hide resolved
<button onClick={handleClick}>{name}</button>
</li>
<hr />
<MdOutlineDeleteForever className="text-red-700" onClick={handleDelete} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

The delete button should be commented out as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

check again working now

Comment on lines 8 to 27
@apply rounded-2xl shadow-2xl px-4 md:px-24 text-black mt-8;
}
#list {
@apply mt-8 flex flex-col justify-center items-center;
}
.input {
@apply border border-blue-700 text-black bg-white p-3 rounded-xl;
width: 70%;
}
.btn {
background-color: #113767;
@apply text-white p-3 rounded-xl shadow-md mb-8;
}
:hover .btn {
background-color: #38598b;
}
.exit-btn {
background-color: yellow;
color: black;
@apply p-3 rounded-xl shadow-md;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice use of the @apply directive to make your code more readable 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thank u


export function Home({ data, setListPath, userId, userEmail }) {
const [name, setName] = useState('');
const [selectedList, setSelectedList] = useState(null); // State to track the selected list
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not need this additional state here. We already have the listPath in the App, you can pass it as props and use it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay... thank you

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

setListPath={setListPath}
setListPath={handleListClick}
deleteList={deleteList} // the deleteList function is being passed as a prop
selected={selectedList === item.path} // Checks if the list is selected
Copy link
Collaborator

Choose a reason for hiding this comment

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

as I said above instead of the selectedList, use the lisrPath from the parent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@ijayhub
Copy link
Collaborator Author

ijayhub commented Mar 30, 2024

I think it looks good, the list can be a bit more centered. I think the background should be just one solid color like #A2A8D3

I think a better title is "Create a shopping list"

button can be "Create new list"

and last button can be "Edit existing list"

Answer:

  1. It is centers check on small screen
  2. I just wanted d homepage visually appealing last demo MJ mention that it was all blue.
  3. I updated d U.I
  4. For d last button it is just for beauty have changed d text description.

@ijayhub ijayhub closed this Mar 30, 2024
@ijayhub ijayhub reopened this Mar 30, 2024
Comment on lines 35 to 39
// to create a highlighted list
const handleListClick = (path) => {
setListPath(path);
setSelectedList(path); // this update and highlight the list path
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should keep the handleListCLick, only remove the duplicate setSelectedList(path)

Comment on lines +59 to 60
listPath={listPath} // Pass listPath to SingleList
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need to pass the listPath further, just use it here.
selected={listpath === item.path}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I dont really understand...

Copy link
Collaborator

Choose a reason for hiding this comment

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

before you were using another duplicated state selectedList to check if the list is selected like this :
selected={selectedList === item.path}.

Now you will use the listPath, that's it:
selected={listpath === item.path}

@jmahamed jmahamed merged commit 8665b57 into main Mar 31, 2024
2 checks passed
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.

4 participants