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

Tweaks #41

Merged
merged 56 commits into from
Aug 9, 2024
Merged

Tweaks #41

merged 56 commits into from
Aug 9, 2024

Conversation

itexpert120
Copy link
Collaborator

@itexpert120 itexpert120 commented Aug 8, 2024

Improve Lightouse Scores for Project Page, Category Page, Bookmarks Page
Fix misc bugs

EDIT: TOTAL REFACTOR

Copy link

vercel bot commented Aug 8, 2024

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

Name Status Preview Comments Updated (UTC)
near-directory-nextjs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 3:45pm

@elliotBraem
Copy link
Contributor

Vercel deployment is failing, probably because of trying to access client-side window in a server side component

Screenshot 2024-08-08 at 6 35 40 PM

<ProjectCard key={pid} project={projects[pid]} maxWidth />
))}
{loading
? Array.from({ length: 4 }).map((_, index) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

I felt it was smoother before -- and I think the loading section showing project skeletons -> "no bookmarks found" is a a bit jarring; seems to take longer to load too with loading state defaulting to true.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, thinking more about it -- especially since this is dependent on local storage starred projects, rather than the "fetchAllProjects" -- we shouldn't be getting hung up on this fetch. Either there are no starredProjects, or there are and then we need fetch

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, i'll revert

return (
<div className="container relative mx-auto">
<input
ref={inputRef}
type="text"
name="search"
value={searchKey}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I don't know for certain, but isn't this the same as

<input autoFocus onFocus={(e) => e.target.select()} />?

@@ -1,22 +1,40 @@
"use client";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was "use client" introduced? Is it because of the useSearchStore?

An alternative may be to keep the useSearchStore outside and have this component take a loading prop. Keep it as "projects.length === 0" shows none found. Then set loading in whatever is calling it -- I see there was a bit of a hack put in for category-project-list to make this work

@elliotBraem elliotBraem merged commit 44298d8 into main Aug 9, 2024
4 checks passed
@elliotBraem elliotBraem deleted the tweaks branch August 9, 2024 15:50
@elliotBraem elliotBraem mentioned this pull request Aug 9, 2024
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.

2 participants