Improve "gallery view" of series block #737
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now the individual video items are not created at the top anymore and are passed down via `children`. Instead a more well-typed data structure is passed down. Further, there exist one component for each of the three view modes now. That gives a bit more flexibility as each one can be changed fairly independently from one another.
This should look better on more screens. Before, there was some left-over space on the right if there wasn't enough space for a new video item. On some screens it looked really bad because a new item would almost fit. With this, the items themselves scale a bit to fill the space. They do have a max width. When that's reached, the remaining space is distributed as margin left/right of the items.
owi92
approved these changes
Mar 13, 2023
There was a problem hiding this comment.
The refactor makes a lot of sense and with it the code looks much tidier and easier to maintain. I did not notice any issues arising from this.
I also really like the change you made to the gallery view. I definitively think this is an improvement and I don't find the max-width overwhelming at all. It works and looks pretty good on almost all screen sizes in every major browser.
However I do find that the gap between two videos can become pretty wide (really only noticeable on screen widths between sth like 1040 and 1080 and also around 780). This is just my subjective opinion and isn't a dealbreaker for me, though I wouldn't mind if this gets tweaked a little.
LukasKalbertodt
force-pushed
the
better-grid
branch
from
e359225
to
2dc0f89
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the second commit message for the main attraction:
The first commit refactors those components to make the second commit possible. The refactoring makes it so that all "view dependent" CSS code is actually contained in the respective
*Viewcomponent, not conditionally insideItem.Regarding the main style change: I'm really interested in just feedback. The video items certainly grew in size on average. It's still smaller than on YouTube. I'm not sure if the max width is too overwhelming? I'm also not sure if distributing the left-over space to each item instead of all at the end is an improvement? I'm open to suggestions. Of course, this is something that can be fine-tweaked a lot to make certain screen sizes look better.