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.

Sign up for GitHub

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

Hide MagnifyHint when image is already fully magnified, and use magnified image in Lightbox #50

Merged
merged 3 commits into from
Mar 6, 2020
Merged

Hide MagnifyHint when image is already fully magnified, and use magnified image in Lightbox #50

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b46ddaf
Hide MagnifyHint when image is already fully magnified, and use magni…
kevhender Feb 24, 2020
37741be
Reset window width for future tests
kevhender Feb 24, 2020
de677b7
Add explanation for media width check
kevhender Mar 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/carousel/Carousel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ const Carousel = React.forwardRef((props, ref) => {
slideStyle={slideStyle}
autoplay={autoplay}
interval={interval}
containerStyle={{ alignItems: 'center' }}
>
{children}
</AutoPlaySwipeableViews>
Expand Down
32 changes: 22 additions & 10 deletions src/carousel/MediaCarousel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ function MediaCarousel(props) {
const [lightboxActive, setLightboxActive] = useState()
const theme = useTheme()
const isSmall = useMediaQuery(theme.breakpoints.down('xs'))
const isTouchScreen = useMediaQuery('(hover:none)')

useEffect(() => {
// Reset selection index when media changes
Expand Down Expand Up @@ -167,14 +168,24 @@ function MediaCarousel(props) {
}

if (media && media.full && media.full.some(item => item.magnify)) {
belowAdornments.push(
<MagnifyHint
key="magnify-hint"
over={over}
disableExpand={lightboxActive}
className={magnifyHintClassName}
/>,
)
let showHint = true
if (typeof window !== 'undefined') {
const { innerWidth } = window
const mediaWidth = get(media.full[selected], 'magnify.width', 10000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a variable for this magic 10000?

if (mediaWidth <= innerWidth) {
showHint = false
}
}
if (showHint) {
belowAdornments.push(
<MagnifyHint
key="magnify-hint"
over={over}
disableExpand={lightboxActive}
className={magnifyHintClassName}
/>,
)
}
}

if (lightboxActive) {
Expand Down Expand Up @@ -212,9 +223,10 @@ function MediaCarousel(props) {
onLoad={i === 0 ? onFullSizeImagesLoaded : null}
magnifyProps={magnifyProps}
{...item}
magnify={isSmall ? undefined : item.magnify}
magnify={isTouchScreen ? undefined : item.magnify}
src={get(item, 'magnify.src', item.src)}
imageProps={
lightboxActive && !isSmall
lightboxActive && !isTouchScreen
? {
fill: false,
contain: true,
Expand Down
27 changes: 27 additions & 0 deletions test/carousel/MediaCarousel.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ describe('MediaCarousel', () => {

const mockMediaQuery = jest.spyOn(useMediaQuery, 'default')

beforeEach(() => {
window.innerWidth = 1024
window.dispatchEvent(new Event('resize'))
})

afterEach(() => {
wrapper.unmount()
})
Expand Down Expand Up @@ -114,6 +119,13 @@ describe('MediaCarousel', () => {
contain: true,
src: media.full[0].magnify.src,
})

expect(
wrapper
.find(Media)
.first()
.prop('src'),
).toEqual(media.full[0].magnify.src)
})

it('should pass height 100% to Carousel if window size is small and lightbox is opened', () => {
Expand All @@ -130,6 +142,21 @@ describe('MediaCarousel', () => {
).toBe('100%')
})

it('should hide the magnify hint if the lightbox is opened and the window is wider than the image', () => {
wrapper = mount(<MediaCarousel media={media} />)
wrapper.find(Carousel).simulate('click')
console.log(wrapper.find(MagnifyHint))
expect(wrapper.find(MagnifyHint)).toExist()

window.innerWidth = 1300
window.dispatchEvent(new Event('resize'))

wrapper = mount(<MediaCarousel media={media} />)
wrapper.find(Carousel).simulate('click')
console.log(wrapper.find(MagnifyHint))
expect(wrapper.find(MagnifyHint)).not.toExist()
})

it('should pass thumbnail to render below the Carousel if image is not loaded and thumbnail is passed as prop ', () => {
wrapper = mount(<MediaCarousel media={media} thumbnail={{ test: 'test' }} />)

Expand Down