Skip to content

feat: comic issue, comic series and library page updates#88

Open
khubak wants to merge 62 commits intodevfrom
feat/hero-image-updates
Open

feat: comic issue, comic series and library page updates#88
khubak wants to merge 62 commits intodevfrom
feat/hero-image-updates

Conversation

@khubak
Copy link
Copy Markdown
Contributor

@khubak khubak commented Jan 13, 2025

No description provided.

@khubak khubak requested a review from josip-fundl January 13, 2025 11:56

if (!me) return null

const favoriteComics = await fetchFavoriteComics({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would like to avoid this approach.
We don't need to fetch favorite comics if we are on creators tab and vice versa.

Also you are missing fetch for owned comics here/owned issues, right? Or that's done with the purpose to keep it as it was?

Copy link
Copy Markdown
Contributor Author

@khubak khubak Jan 14, 2025

Choose a reason for hiding this comment

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

that fetch is outside of the wrapper as content of that tab changes (depending on route, ergo dreader.app/library and dreader.app/library/[slug])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will implement a tab listener together with React Query that will enable fetching only if the pressed tab is selected.

<LibraryTabs comicIssue={comicIssue} ownedIssues={ownedIssues} />
</BaseLayout>
<LibraryTabsWrapper userId={me.id}>
<TabsContent className='mt-0 pt-4 border-t border-grey-300' value='owned'>
Copy link
Copy Markdown
Contributor

@luka-fundl luka-fundl Jan 15, 2025

Choose a reason for hiding this comment

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

can we move tabs content for owned issues inside LibraryTabsWrapper?

Copy link
Copy Markdown
Contributor Author

@khubak khubak Jan 15, 2025

Choose a reason for hiding this comment

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

yes, i would like to do that together with infinite scroll + fetch per tab clicked ticket since that one will kind of decide on final structure.

{comicIssue && ownedIssues.length ? (
<OwnedIssuesContent comicIssue={comicIssue} ownedIssues={ownedIssues} />
) : (
<EmptyLibrarySection
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In my opinion, we should move Empty section inside OwnedIssuesContent for this use case.

We could check ownedIssues.length over there and display empty section accordingly.
Thoughts?

</BaseLayout>
<LibraryTabsWrapper userId={me.id}>
<TabsContent className='mt-0 pt-4 border-t border-grey-300' value='owned'>
{ownedComics.length ? (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it possible to move this logic inside LibraryTabsWrapper?

{ownedComics.length ? (
<ComicsByAlphabet comics={ownedComics} ComicCard={OwnedComicCard} />
) : (
<EmptyLibrarySection
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same comment as above for owned issues.length

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.

3 participants