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

fixed: small images for scaling problem #8761

Merged
merged 16 commits into from
Jul 11, 2022
Merged

fixed: small images for scaling problem #8761

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
25fa661
small images for scaling problem fixed
metehanozyurt Apr 24, 2022
d2715fc
Unnecessary lines deleted
metehanozyurt Apr 25, 2022
184e4ca
Unnecessary conditions and callbacks deleted
metehanozyurt Apr 26, 2022
71be3ee
in setImageRegion function imageHeight condition added
metehanozyurt Apr 30, 2022
2bc6fa6
reverted change
metehanozyurt May 13, 2022
f4c50c9
Merge branch 'main' of https://github.com/Expensify/App into issue-8471
metehanozyurt May 22, 2022
fe81aaa
removed thumbnail from native code when loading, condition has been a…
metehanozyurt May 22, 2022
b05db83
setScale method added.
metehanozyurt May 24, 2022
aee1d1e
Removed unnecessary variable assignments.
metehanozyurt May 24, 2022
5aa5749
zoomScale calculation on mWeb added and when loading images doesnt sh…
metehanozyurt May 31, 2022
bf03160
typo fixed
metehanozyurt May 31, 2022
ee96405
comment changed
metehanozyurt Jun 4, 2022
eb3d169
comments became more understandable.
metehanozyurt Jun 17, 2022
25ab7dc
Comments moved before prop.
metehanozyurt Jun 20, 2022
0dc63b0
Merge branch 'main' of https://github.com/Expensify/App into issue-8471
metehanozyurt Jul 6, 2022
43876b5
heights checked in setScale function
metehanozyurt Jul 10, 2022
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
Prev Previous commit
Next Next commit
typo fixed
  • Loading branch information
@metehanozyurt
metehanozyurt committed May 31, 2022
commit bf0316092ded48e3816697322503b2eb102aedcb
4 changes: 2 additions & 2 deletions src/components/ImageView/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ class ImageView extends PureComponent {
style={this.state.zoomScale === 0 ? undefined : [
styles.w100,
styles.h100,
]} // Hide image until zoomScale scale calculated
]} // Hide image until zoomScale calculated
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
]} // Hide image until zoomScale calculated
]} // Hide image until zoomScale calculated to prevent showing preview with wrong dimensions.

resizeMode={this.state.zoomScale > 1 ? 'center' : 'contain'} // For big dimension images 'contain' works much effective
Copy link
Member

Choose a reason for hiding this comment

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

Improve this comment. You should tell how that is more effective and what is the effect.

Copy link
Member

Choose a reason for hiding this comment

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

Best thing to do would be find why image is all stretched on mWeb while it work great on Web.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That code doesn't work on the web.
The Image component is inside the Pressable component on desktop and Web.

Copy link
Member

@parasharrajat parasharrajat Jun 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I meant to ask why image is all stretched on mWeb while it work great on Web for contain value.
Why contain is not working on mWeb same as web?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the web side, there is a pressable component on the upper level of the image component. When small size photos come, it changes the style information and keeps it in the middle of the screen. It does this by not giving the entire container space to the Image component.
But in mWeb, Image component has all the space. I can do it like on the web, if I write the code below, it works. But I think we would have done an extra calculation. The current version of PR is actually doing this job for us.

<Pressable
        style={{
                            ...StyleUtils.getZoomSizingStyle(this.state.isZoomed, this.state.imgWidth, this.state.imgHeight, this.state.zoomScale,
                                this.state.containerHeight, this.state.containerWidth),
                            ...StyleUtils.getZoomCursorStyle(this.state.isZoomed, this.state.isDragging),
                            ...this.state.isZoomed && this.state.zoomScale >= 1 ? styles.pRelative : styles.pAbsolute,
                            ...styles.flex1,
                        }}
                    >
                        <Image
                            source={{uri: this.props.url}}
                            style={this.state.zoomScale === 0 ? undefined : [
                                styles.w100,
                                styles.h100,
                            ]} // Hide image until zoomScale calculated
                            resizeMode="contain"
                            onLoadStart={this.imageLoadingStart}
                            onLoadEnd={this.imageLoadingEnd}
                        />
                        {this.state.isLoading && (
                            <FullscreenLoadingIndicator
                                style={[styles.opacity1, styles.bgTransparent]}
                            />
                        )}
                    </Pressable>

Copy link
Member

Choose a reason for hiding this comment

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

Ok, then leave this

onLoadStart={this.imageLoadingStart}
onLoadEnd={this.imageLoadingEnd}
Expand Down Expand Up @@ -250,7 +250,7 @@ class ImageView extends PureComponent {
style={this.state.zoomScale === 0 ? undefined : [
styles.h100,
styles.w100,
]} // Hide image until zoomScale scale calculated
]} // Hide image until zoomScale calculated
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
]} // Hide image until zoomScale calculated
]} // Hide image until zoomScale calculated to prevent showing preview with wrong dimensions.

resizeMode="contain"
onLoadStart={this.imageLoadingStart}
onLoadEnd={this.imageLoadingEnd}
Expand Down