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

Show movies from watch list in person detail page #545

Merged
merged 6 commits into from
Nov 12, 2023

Conversation

sahinakkaya
Copy link
Contributor

Fixes #543

I mostly copied existing code and adjusted it to show movies from watch list. I tested it for my user and it works for me.

@sahinakkaya
Copy link
Contributor Author

sahinakkaya commented Nov 11, 2023

After seeing this in action, I think it might be a good idea to also show the same information for directors. I can add it too. Let me know what you think.

Edit: Added it :D I think this version is better but I can revert.

@sahinakkaya
Copy link
Contributor Author

Some screenshots:
resim

resim

@leepeuker
Copy link
Owner

leepeuker commented Nov 11, 2023

What about showing all movies together (losing the "From your watchlist") and shade the posters of movies not yet watched or something like that? 🤔 If we add e.g. general playlists later we probably want to include movies which are on an users playlist here too.. If we add e.g. general playlists later we probably want to include movies which are on an users playlist here too.

The whole person page is a construction site, so all ideas are welcome

Edit: Oh and thanks for your first pull request! 🎉

@leepeuker
Copy link
Owner

What about something like this. We could reuse the filter logic from the other sites and edit it so that you can select to e.g. only show watched, on watch list, etc or select how the movies should be ordered

image

@sahinakkaya
Copy link
Contributor Author

Yeah, these ideas sound good too. The bad news is I don't know if can implement them :D (sure I can but I mean if I can do it in a reasonable amount of time).

Btw, the whole point of opening this pull request for me is to be able to easily see the movies from watch list at first glance. So I am not sure how we can make this possible with filtering. Oh... What about combining both of your ideas? Show a filter box, allow users to filter movies based on them being in watch list, history or other custom lists AND shade the posters of movies not yet watched?

@leepeuker
Copy link
Owner

Have a look at my comments please. Afterwards I will approve the PR. The layout will probably change again, maybe to something like I suggested here, but who knows when. So in the meantime this is a great solution to already see the watchlist movies 👍

Co-authored-by: Lee Peuker <lee.peuker@protonmail.com>
@sahinakkaya
Copy link
Contributor Author

OK, done!

@sahinakkaya sahinakkaya marked this pull request as draft November 12, 2023 11:33
@sahinakkaya
Copy link
Contributor Author

Converted it to draft because of #545 (comment)

Apparently, sub-query is required in sql statement.
See leepeuker#545 (comment)
@sahinakkaya sahinakkaya marked this pull request as ready for review November 12, 2023 11:40
Co-authored-by: Lee Peuker <lee.peuker@protonmail.com>
@sahinakkaya
Copy link
Contributor Author

Uhm... when is this going to be merged? I can't wait to use it in my server :)

@leepeuker leepeuker merged commit 35de7b9 into leepeuker:main Nov 12, 2023
1 check 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.

Show movies from both watch history and watch list in person detail page
2 participants