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

feat: saved resources page #2211

Merged
merged 14 commits into from
Jan 20, 2024
Merged

feat: saved resources page #2211

merged 14 commits into from
Jan 20, 2024

Conversation

Vidip-Ghosh
Copy link
Contributor

Fixes Issue

Closes #2174

Changes proposed

I've started to work on Saved new resources page.
Its in /saved route.

Screenshots

image

Note to reviewers

Copy link

vercel bot commented Dec 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
linkshub ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 19, 2024 8:03am

Copy link

@github-actions github-actions bot 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, Vidip-Ghosh, for creating this pull request and contributing to LinksHub! 💗

The maintainers will review this Pull Request and provide feedback as soon as possible! 😇
We appreciate your patience and contribution, Keep up the great work! 😀

@Vidip-Ghosh Vidip-Ghosh changed the title Dev feat: saved resources page Dec 22, 2023
Copy link
Collaborator

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Hi @Vidip-Ghosh. I went to the Saved section and it showed me this Screenshot of saved tab not appearing.

@Vidip-Ghosh
Copy link
Contributor Author

Hi @Vidip-Ghosh. I went to the Saved section and it showed me this Screenshot of saved tab not appearing.

In sidebar component, I replaced Link with <a href=""
So it started to work.

Copy link
Collaborator

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Looks good on my end, but I want to see what @Anmol-Baranwal & @aftabrehan thinks. You too @rupali-codes.

Copy link
Collaborator

@Anmol-Baranwal Anmol-Baranwal left a comment

Choose a reason for hiding this comment

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

It needs some styling changes as in figma.

image

Maybe provide a screenshot of UI, so that I can review it.

@CBID2 CBID2 linked an issue Dec 22, 2023 that may be closed by this pull request
@CBID2 CBID2 added goal: new-design Design related issues priority: medium Modifying an existing feature priority: high Making completely new feature labels Dec 22, 2023
Copy link
Collaborator

@aftabrehan aftabrehan left a comment

Choose a reason for hiding this comment

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

Excellent PR, @Vidip-Ghosh 💪 🚀 💯

A few adjustments are required and then we're good to go 🚀

components/Sidebar/Sidebar.tsx Show resolved Hide resolved
pages/saved.tsx Outdated Show resolved Hide resolved
public/SaveRemove.png Outdated Show resolved Hide resolved
@Vidip-Ghosh
Copy link
Contributor Author

Vidip-Ghosh commented Dec 23, 2023

Screenshot 2023-12-23 at 21 21 02 @aftabrehan In dev branch, I am getting this error due to which I am unable to debug. How can I solve?

@aftabrehan
Copy link
Collaborator

Screenshot 2023-12-23 at 21 21 02 @aftabrehan In dev branch, I am getting this error due to which I am unable to debug. How can I solve?

It seems this error is triggered by a missing dependency. Could you please try to run pnpm install after switching the dev branch?

@Vidip-Ghosh
Copy link
Contributor Author

Screenshot 2023-12-23 at 21 21 02 @aftabrehan In dev branch, I am getting this error due to which I am unable to debug. How can I solve?

It seems this error is triggered by a missing dependency. Could you please try to run pnpm install after switching the dev branch?

Screenshot 2023-12-23 at 21 37 31 I ran but it shows up to date & still the same error.

@aftabrehan
Copy link
Collaborator

@Vidip-Ghosh, Please, run a clean build of your Next.js project. This involves deleting the .next directory and then restarting your development server.

rm -rf .next
pnpm run dev

@Vidip-Ghosh
Copy link
Contributor Author

It needs some styling changes as in figma.

image

Maybe provide a screenshot of UI, so that I can review it.

Screenshot 2023-12-24 at 22 28 35

Figma UI of saved resources

@Anmol-Baranwal
Copy link
Collaborator

Figma UI of saved resources

I was saying, it wasn't exactly the same as the interface design in figma. You need to do some styling changes.
You need to give a screenshot of your code UI.

@Vidip-Ghosh
Copy link
Contributor Author

Figma UI of saved resources

I was saying, it wasn't exactly the same as the interface design in figma. You need to do some styling changes. You need to give a screenshot of your code UI.

Screenshot 2023-12-26 at 10 45 13

Copy link
Owner

@rupali-codes rupali-codes left a comment

Choose a reason for hiding this comment

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

@Vidip-Ghosh its looking too sticky, can you add a little space around the heading as in the design?

image

@Vidip-Ghosh
Copy link
Contributor Author

@Vidip-Ghosh its looking too sticky, can you add a little space around the heading as in the design?

image

Yes I'll

@Vidip-Ghosh
Copy link
Contributor Author

Screenshot 2023-12-29 at 10 38 57 @rupali-codes I hope this much space is alright

Copy link

vercel bot commented Dec 29, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @rupali-codes on Vercel.

@rupali-codes first needs to authorize it.

Copy link
Collaborator

@Anmol-Baranwal Anmol-Baranwal left a comment

Choose a reason for hiding this comment

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

🚀 Looks good to me.

@aftabrehan @rupali-codes @CBID2
Please review it.

Copy link
Collaborator

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Looks awesome! 😎 Your turn @rupali-codes or @aftabrehan

Copy link
Collaborator

@aftabrehan aftabrehan left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍

It just needs minor adjustments and then we're good to go 🚀

pages/saved.tsx Outdated Show resolved Hide resolved
pages/saved.tsx Outdated Show resolved Hide resolved
pages/saved.tsx Outdated Show resolved Hide resolved
@aftabrehan aftabrehan added the status: ready-to-merge Approved & its ready-to-merge label Jan 18, 2024
@aftabrehan
Copy link
Collaborator

@rupali-codes, could you assist in authorizing the deployment? This way, we can review the latest changes.

Copy link
Collaborator

@Anmol-Baranwal Anmol-Baranwal left a comment

Choose a reason for hiding this comment

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

🚀 Looks good to me.

Copy link
Owner

@rupali-codes rupali-codes left a comment

Choose a reason for hiding this comment

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

lgtm

@rupali-codes rupali-codes merged commit 7f551ce into rupali-codes:dev Jan 20, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal: new-design Design related issues priority: high Making completely new feature priority: medium Modifying an existing feature status: ready-to-merge Approved & its ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add new design for saved resources
5 participants