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

Sort items in people view #670

Merged
merged 4 commits into from
Nov 5, 2024
Merged

Sort items in people view #670

merged 4 commits into from
Nov 5, 2024

Conversation

AlexPerathoner
Copy link
Contributor

@AlexPerathoner AlexPerathoner commented Oct 23, 2024

Relates to #619

Changes made

Added sort dropdown to sort items in /person/[id] views by Vote count (former method), oldest / newest release date

immagine

Copy link
Member

@IRHM IRHM left a comment

Choose a reason for hiding this comment

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

Hi @AlexPerathoner, thank you for taking the time to do this!

I noticed one bug with the sorting and one small thing I think we could improve upon (we could possibly be using just one list instead of two for storing the credits).

Let me know if I can help with anything (or if you want me to finish any of this)!

src/routes/(app)/person/[id]/+page.svelte Outdated Show resolved Hide resolved
src/routes/(app)/person/[id]/+page.svelte Outdated Show resolved Hide resolved
src/routes/(app)/person/[id]/+page.svelte Outdated Show resolved Hide resolved
src/routes/(app)/person/[id]/+page.svelte Show resolved Hide resolved
src/routes/(app)/person/[id]/+page.svelte Outdated Show resolved Hide resolved
@IRHM IRHM added the enhancement New feature or request label Oct 27, 2024
@AlexPerathoner
Copy link
Contributor Author

Removed the two cached arrays into just one; updated methods to reflect that; updated sort method to treat shows which don't have a release date nor a first air date as unreleased (so they appear first when sorting by Newest and last when by Oldest).

Also noticed that some people have duplicate items in them (because of different character and credits_id (for example https://api.themoviedb.org/3/person/25072/combined_credits has 59941 twice, once as Self and once as Self - Guest). Since in the people view there's no reference to the played character yet, I added a filter to remove these duplicates.

… .container

- Removed min-width so improve responsivity for mobile (overflows).
- Removed unused styles (margin-top/bottom that were overridden in .container).
- Add max-width to match PosterList.
- Remove .container and add styles to .filters (combined them).
No need for the filter when this is true, we can just pass the options var.
…credits array directly

I believe this should be more performant (removing the use of spread operator) and is just a little cleaner.
Copy link
Member

@IRHM IRHM left a comment

Choose a reason for hiding this comment

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

This is awesome, thank you!

I'm liking the less duplicates in the person view, it has always annoyed me!

p.s. I made a few tiny changes, didn't find any bugs, just changed around some code/styling.

@IRHM IRHM merged commit 27218e8 into sbondCo:dev Nov 5, 2024
1 check passed
@IRHM IRHM mentioned this pull request Nov 5, 2024
@IRHM IRHM added this to the v1.45 milestone Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Waiting Until Dev Build
Development

Successfully merging this pull request may close these issues.

2 participants