Skip to content

Fix ThumbnailImage on mWeb #23392

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

Merged
AndrewGable merged 3 commits into from
Jul 27, 2023
Merged

Fix ThumbnailImage on mWeb #23392

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2174581
Fix ThumbnailImage on mWeb
ginsuma Jul 22, 2023
6f219d3
Change variable name
ginsuma Jul 26, 2023
a61d229
Add comment
ginsuma Jul 26, 2023
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
7 changes: 5 additions & 2 deletions src/components/ThumbnailImage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import lodashClamp from 'lodash/clamp';
import React, {PureComponent} from 'react';
import {View} from 'react-native';
import {View, Dimensions} from 'react-native';
import PropTypes from 'prop-types';
import ImageWithSizeCalculation from './ImageWithSizeCalculation';
import styles from '../styles/styles';
import * as StyleUtils from '../styles/StyleUtils';
import withWindowDimensions, {windowDimensionsPropTypes} from './withWindowDimensions';
import * as DeviceCapabilities from '../libs/DeviceCapabilities';

const propTypes = {
/** Source URL for the preview image */
Expand Down Expand Up @@ -62,7 +63,9 @@ class ThumbnailImage extends PureComponent {
// Note: Clamp minimum width 40px to support touch device
let thumbnailScreenWidth = lodashClamp(width, 40, 250);
const imageHeight = height / (width / thumbnailScreenWidth);
let thumbnailScreenHeight = lodashClamp(imageHeight, 40, this.props.windowHeight * 0.4);
// On mWeb, when soft keyboard opens, window height changes, making thumbnail height inconsistent. We use screen height instead.
const screenHeight = DeviceCapabilities.canUseTouchScreen() ? Dimensions.get('screen').height : this.props.windowHeight;

@rushatgabhane rushatgabhane Jul 26, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ginsuma could you please add a comment above this line explaining why we're using Dimensions and not the HOC withWindowDimensions for touchscreen devices

@ginsuma ginsuma Jul 26, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"On mWeb, when soft keyboard opens, window height changes, making thumbnail height inconsistent. We use screen height instead."
Is it a good explain?
cc: @rushatgabhane

let thumbnailScreenHeight = lodashClamp(imageHeight, 40, screenHeight * 0.4);
const aspectRatio = height / width;

// If thumbnail height is greater than its width, then the image is portrait otherwise landscape.
Expand Down