Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-14500 Fix for pop caused by size_aware_image #2469

Merged
merged 3 commits into from
Mar 20, 2019

Conversation

sudheerDev
Copy link
Contributor

@sudheerDev sudheerDev commented Mar 8, 2019

Summary

Removes scroll pop caused by size_aware_Image component

  • Use svg for creating placeholder instead of canvas and dataUrl
  • onerror should just be referencing the class method instead of executing on assignment

screenshots with breakpoint onMount
Before
Screenshot 2019-03-08 at 2 47 33 PM
After
Screenshot 2019-03-09 at 1 08 05 AM

Ticket Link

MM-14500
MM-14637
MM-13908
MM-13158
MM-13930

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed
  • Added or updated unit tests (required for all new features)

@sudheerDev sudheerDev added the 2: Dev Review Requires review by a core commiter label Mar 8, 2019
@sudheerDev sudheerDev requested review from hmhealey and enahum March 8, 2019 20:04
@sudheerDev sudheerDev added Work in Progress Not yet ready for review and removed 2: Dev Review Requires review by a core commiter labels Mar 8, 2019
@sudheerDev
Copy link
Contributor Author

Need a minor css change

@sudheerDev sudheerDev added 2: Dev Review Requires review by a core commiter Work in Progress Not yet ready for review and removed Work in Progress Not yet ready for review 2: Dev Review Requires review by a core commiter labels Mar 8, 2019
@sudheerDev sudheerDev force-pushed the MM-14500 branch 2 times, most recently from 9d7d61a to d5d126c Compare March 11, 2019 09:28
@sudheerDev sudheerDev added 2: Dev Review Requires review by a core commiter and removed Work in Progress Not yet ready for review labels Mar 11, 2019
@sudheerDev
Copy link
Contributor Author

@hmhealey Up for review.

…sts (mattermost#2334)

* MM-13099 Use size_aware_image component for all instances for loading images in webapp.
* Removing getFileDimensionsForDisplay in favour of  SizeAwareImage as it would be reliable in preventing
  scroll pop
* Add a fallback for svg as older version do not have dimentions
MM-14500 Fix for pop caused by size_aware_image

  * Use svg for creating placeholder instead of canvas and dataUrl
    Change to inline-block for hover css shadow effect
    Add defensive check for dimensions

Removing display: inline-flex as it causes svg in the placeholder to not take the available space
Have to move border hioghlight to img tags as div is display block by default so shows border for the entire
box instead of just the image

Update snapshots and fix lint
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Awesome idea. The generated image felt really hacky when I added it, and this seems much nicer.

components/size_aware_image.jsx Outdated Show resolved Hide resolved
components/size_aware_image.jsx Outdated Show resolved Hide resolved
components/size_aware_image.jsx Outdated Show resolved Hide resolved
@hmhealey hmhealey removed the 2: Dev Review Requires review by a core commiter label Mar 20, 2019
@hmhealey hmhealey added this to the v5.10.0 milestone Mar 20, 2019
@hmhealey hmhealey merged commit ee41ff2 into mattermost:master Mar 20, 2019
@sudheerDev sudheerDev deleted the MM-14500 branch March 20, 2019 14:51
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Mar 20, 2019
stevepartridge pushed a commit to stevepartridge/mattermost-webapp that referenced this pull request Mar 30, 2019
* MM-13099 Use size_aware_image component for when loading images in posts (mattermost#2334)
* MM-13099 Use size_aware_image component for all instances for loading images in webapp.
* Removing getFileDimensionsForDisplay in favour of  SizeAwareImage as it would be reliable in preventing
  scroll pop
* Add a fallback for svg as older version do not have dimentions
MM-14500 Fix for pop caused by size_aware_image

  * Use svg for creating placeholder instead of canvas and dataUrl
    Change to inline-block for hover css shadow effect
    Add defensive check for dimensions

Removing display: inline-flex as it causes svg in the placeholder to not take the available space
Have to move border hioghlight to img tags as div is display block by default so shows border for the entire
box instead of just the image

Update snapshots and fix lint

* Fix review comments
@DHaussermann DHaussermann added the Tests/Done Release tests have been written label Apr 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Done Release tests have been written
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants