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

[imagetool] Scale down large images before passing to ImageTool #567

Merged
merged 3 commits into from
Feb 5, 2018

Conversation

bjoerge
Copy link
Member

@bjoerge bjoerge commented Feb 5, 2018

This will scale down large images before passing on to the ImageTool component. It's a defensive fix to ensure the imagetool works even if you give it a huge image.

This is just an internal refactoring, the public API stays the same.

It is also a good idea to scale down the images before fetching from the CDN, but that's a separate task.

@bjoerge bjoerge force-pushed the imagetool-resize-large-images branch from ac824ca to a6c7f05 Compare February 5, 2018 09:05
import PropTypes from 'prop-types'
import React from 'react'

export default class ImageLoader extends React.Component {
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect class name

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks!

children: PropTypes.func.isRequired
}

getCanvas() {
Copy link
Member

Choose a reason for hiding this comment

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

Won't this leak a new canvas into the document for every Resize instance?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, I had implemented a cWU that removed it, but it got missing in action obviously. Added it back now. Thanks!

@bjoerge bjoerge merged commit 469569c into next Feb 5, 2018
@bjoerge bjoerge deleted the imagetool-resize-large-images branch February 5, 2018 10:56
kristofferjs pushed a commit that referenced this pull request Feb 12, 2018
* [imagetool] Scale down large images before passing to ImageTool

* [imagetool] Remove canvas element on cWU

* [imagetool] Fix incorrect class name
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