-
-
Notifications
You must be signed in to change notification settings - Fork 840
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
Add zoom slider to other grid views #4674
Add zoom slider to other grid views #4674
Conversation
Hmm, I was under the impression that the zoom level wasn't stored in saved filters, but that does not seem to be the case. I don't think anything else needs to be done here. |
I can see merit in a zoom slider for movie cards, but is there really a need for zooming of performer and studio cards? |
I remember Echo specifically requested the zoom slider on the performer page so he could fit more of those cards on his screen https://discord.com/channels/559159668438728723/644934273459290145/1204623730895228989. Everything else was just for consistency. |
Of the new additions, I’d probably only use the slider to reduce the size of the movie cards. I’m not against removing the slider on the studio page, but with the sliders on all the other pages, it seems like we might as well include it. |
Going to reference the FR here so that it's easier to track. |
This branch is up to date with the latest changes in the development branch. |
@WithoutPants, what are your thoughts on closing this PR out on time for the coming release? Sometime after our initial conversation above, you encouraged Gykes to pick up this work, indicating you supported it. Of course, Gykes held off on his change after seeing that this PR existed. However, your continued silence on this despite bringing the PR up on separate occasions leaves me guessing that you do not support this feature. Without knowing where your thoughts are on this pull request, I'm going to go ahead and make the case for the work under the assumption you've changed your mind about considering this pull request. One of the main appeals of having a slider on any page to adjust card sizes rather than hard-coding a set size in the CSS is to support users who view their content on different monitors with varying sizes and resolutions. For this user, being able to quickly adjust the slider based on the monitor in use, rather than tweaking CSS each time, is invaluable. The current experience of having the slider on some pages and not others yields unproportional cards across pages. A user on a large 4K monitor who blew up the scene cards to the max size would likely have some frustrations that the movie and performer cards remain very small to look at. As long as we have the slider on one of the list pages, I can't see a good reason why we wouldn't include it on the other list pages. |
I noticed that at certain window sizes (1280 pixels wide), some slider settings wouldn't change the card size. When I maximised I didn't get this issue. It's a minor issue that can be fixed in a separate PR. |
This pull request brings the zoom slider to the movie, performer, and studio pages. It makes sense to save the state of the zoom index for each page, but to minimize conflicts, it probably makes sense to wait until #4453 is merged.
Closes #2201