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

Add simple image slides with navigation #1415

Closed
wants to merge 7 commits into from

Conversation

roadlittledawn
Copy link
Contributor

@roadlittledawn roadlittledawn commented Jun 21, 2021

Description

Adds simple image slider (without the sliding animation for now) and arrows to navigate to next / prev image.

Does not show nav arrows if only one image

Reviewer Notes

Wasn't sure what looked best for images either clipped on smaller screen or scaled down.

Related Issue(s) / Ticket(s)

Related to:

Screenshot(s)

2021-06-21_14-22-57.mp4

@gatsby-cloud
Copy link

gatsby-cloud bot commented Jun 21, 2021

Gatsby Cloud Build Report

developer-website-develop

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 2m

Performance

Lighthouse report

Metric Score
Performance 🔶 18
Accessibility 🔶 88
Best Practices 🔶 87
SEO 🔶 76

🔗 View full report

@roadlittledawn roadlittledawn changed the base branch from develop to feature/o11y-packs June 21, 2021 21:25
Copy link
Contributor

@zstix zstix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of thoughts.

}
alt="placeholder-text"
css={css`
height: ${height}px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the height does some weird things on mobile:
Screen Shot 2021-06-21 at 2 54 04 PM

And it also looks like it squishes images a bit (depending on the aspect ratio). I wonder if there's a way we can avoid this 🤔
Screen Shot 2021-06-21 at 2 54 59 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i made the images object-fit: contain on mobile screens (smaller than 760px) because seeing the entire image (where you can click and see it in full resolution in another tab) seemed slightly better than seeing small portion of the image where you can't really discern what it is.

i adjusted so it's full width and fixed height on mobile. looks okay, but TBH not sure what best option there is.

for the squishy-ness on those medium-large screens. i adjusted that in the new PR so it doesn't squish / distort so much.

@roadlittledawn
Copy link
Contributor Author

Closing in favor of animated version #1417

@jpvajda jpvajda deleted the clang/image-slider-simple branch September 13, 2021 21:10
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.

2 participants