Image pan and zoom while allowing page to scroll #308
Conversation
Added "Comparing Environments" section and modified selecting environments to say that there is a search box filter.
… as the span scrolls, not the text area, but it works
…ow flex select height. On mouse leave, snap back to fixed height
…less clear pressed. On refresh, if no env was selected before, default to main
|
Sorry, I don't know why so many commits showed up here. It's actually just the last few. The other commits were from my previous pull request and me trying to merge my repo with the official repo (upstream) |
.gitignore
Outdated
| setup.cfg | ||
| py/static/fonts/ | ||
| py/static/css/ | ||
| py/static/js/ |
There was a problem hiding this comment.
The main.js file produced into this folder is actually required for committing, so we probably shouldn't ignore the whole folder
js/ImagePane.js
Outdated
| src={content.src} | ||
| width={Math.ceil(1 + this.props.width * this.state.scale) + "px"} | ||
| height={Math.ceil(1 + this.props.height * this.state.scale) + "px"} | ||
| onWheel={this.handleZoom.bind(this)} |
There was a problem hiding this comment.
If we're changing zoom being scroll dependent this should also be removed right?
There was a problem hiding this comment.
This calls the function that separates the two and is harmless, but unnecessary. I took this out as it is cleaner to just pass the event up to the parent. (behavior remains same whether this line exists or not)
| ref={(ref) => this._windowRef = ref}> | ||
| <div className={barClassNames} | ||
| ref={(ref) => this._barRef = ref}> | ||
| <button title="close" onClick={this.close}>X</button> | ||
| <button title="save" onClick={this.download}>⇩</button> | ||
| <button title="reset" onClick={this.reset} hidden={!this.props.handleReset}>⟲</button> |
There was a problem hiding this comment.
Ah this is great! I can imagine other panes might use this reset feature somewhere down the road. 👍
|
Alright I've gotten a chance to try this out and its awesome. Only thing, can we swap ctrl and shift and check for the pan event before the zoom event? Shift scroll is already overloaded on many browsers as horizontal scroll, so you'll only be able to scroll left and right if using a mouse wheel under the current setting. The ordering is important as well, as shift control scroll should pan left/right rather than zoom |
There was a problem hiding this comment.
@JackUrb is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Added 2 features to the ImagePane Component. The first one allows to zoom on a particular area of the image (while keeping this area in the center of the visible part) The second allows to pan the image with the mouse drag action. Most of the changes are in ImagePane.js. Only two lines of css needed to be changed to allow zooming to work properly. Image interaction was quite poor so far : zoom and pan were not very user-friendly. #308 introduced image pan, but it used onWheel event, which did not allow for horizontal pan on desktop computers. In addition, it was not possible to zoom in a particular area of the image. I tested my changes by compiling the js code, running a visdom server, and displaying a saved visdom environment which included a visdom.image call. I am using a line of javascript which might not be very clean : ` this._paneRef._windowRef.children[1] ` at line 71 of file ImagePane.js, to get the top and left position of the Pane. There may be a cleaner way to get these but I couldn't find. As it uses some "hidden" attributes, it may not be very robust to future changes of React API. My changes should not modify existing features, apart from the zooming and panning speed that I increased in the handleZoom function (feel free to reset these constants to their previous value). I tried to run python example/demo.py but it failed with ```ConnectionRefusedError: [Errno 111] Connection refused```, both with and without my changes (after installing from sources). Not appropriate <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] My code follows the code style of this project. - [x] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [x] For JavaScript changes, I have re-generated the minified JavaScript code. Pull Request resolved: #421 Differential Revision: D8896258 Pulled By: JackUrb fbshipit-source-id: bcdcef3d621c99a3300ecac4246ba06cdd57a397
I was getting annoyed when scrolling down would result in an image getting zoomed in. Also, sometimes, when I would scroll up, the image would zoom out to size zero and the only way to reset was to reload the page. Additionally, there was no way to pan the image after a zoom.
So I did the following: