Color by status #672
Color by status #672
Conversation
AlexPerathoner
force-pushed
the
feature/color-by-status
branch
from
af5cd41 to
10602b5
Compare
prefer pixels, so changed to use that for `bottom`. Calculating width so it can be the same spacing around all edges from 100%.
There was a problem hiding this comment.
Hi @AlexPerathoner thank you for implementing this!
I had some comments on some of the code. One of them I was unsure of if I was testing correctly, if it fixed anything im not seeing?
When I find some more time, I wanted to look into the colors too. I may just tweak around with them to see if we can get them to hide less of the poster while still being legible (like the old color). If not, I may implement a setting to allow toggling this on/off.
| @@ -26,8 +35,8 @@ | |||
| } | |||
| </script> | |||
|
|
|||
| {#if ($page.url?.pathname === "/" || $page.url?.pathname.startsWith("/search/")) && details && dve && dve.length > 0} | |||
There was a problem hiding this comment.
Removing the page check here introduces a bug where:
- Enable the extra details
- Go to another users watched list
- Can see extra details on their page, but can't view to toggle this off/on
I think we could add it back in?
There was a problem hiding this comment.
I see. I had added it in to make the extra details appear on all lists of movies where I could be interested in the color by status. That's also why on the other hand I then added the line above.
I'll search for a different way to do it
There was a problem hiding this comment.
Sounds good @AlexPerathoner, do you think we should just support showing ExtraDetails for posters when viewing other users lists? In that case we could just show the ExtraDetails button in nav on that page too, but I'm not sure if that could get confusing for users (if it's showing, it may seem like it's because it is on their list too), what do you think?
There was a problem hiding this comment.
I personally don't think it would be confusing. After all it's just some extra details on items, not necessarily from their own list. Also with the color they can immediately see if it is indeed on their list
There was a problem hiding this comment.
@AlexPerathoner That sounds good to me
| <div class={`menu${$page.url?.pathname.startsWith("/search/") ? " on-search-page" : ""}`}> | ||
| <div | ||
| class={`menu${$page.url?.pathname.startsWith("/search/") ? " on-search-page" : ""}`} | ||
| style="right: {$page.url?.pathname === '/' || $page.url?.pathname.startsWith('/search/') |
There was a problem hiding this comment.
Was this to fix an issue? (I couldn't find if there was an issue with old code to review)
Changes made
Added background color on Poster based on watch status