-
-
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
Gallery scrubber #5133
Gallery scrubber #5133
Conversation
Tested with zip/non-zip galleries, found no issues. |
<GalleryPreview | ||
gallery={props.gallery} | ||
onScrubberClick={(i) => { | ||
console.log(i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to commit this to production code
); | ||
|
||
return ( | ||
<div className={cx("gallery-card-cover", { portrait: isPortrait })}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed the height and width dimensions were removed from the cover property, and it currently looks like isPortrait
is always set to false. How did you plan to identify when a gallery card has a portrait image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it was a vestigial artifact from the ScenePreview
that it was copied from. As far as I can determine, there's no need for it in the Gallery preview/scrubber, and the styling needs to accommodate for both orientations anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When viewing a gallery card from the gallery page, I agree there isn't much need to make a distinction. I think the value of the portrait class lies in the themeability of those cards when they are used outside of the gallery list. For instance, on the scene details page, the gallery card is currently displayed within the gallery tab, which also displays all the gallery images, making the scrubber redundant. In my Scene details page redesign PR, where a restyled gallery card is used directly in the details tab like is also the case with the movie/groups card, being able to identify the orientation of the cover was essential to determine how to position and size the restyle card to ensure that collapsible section works as intended. I can simply look into calculating the height and width of those images in the front-end code, but I wanted to first raise the value of continuing to store that info on the backend.
Adds a gallery preview scrubber to gallery cards. Like the scene preview scrubber, allows scrubbing through all of the images in a gallery. Clicking on the scrubber navigates to the current image detail page.