Changed slider to a CTRL controlled zooming

[Ren22]

Haven't tested it for mobile devices, but for desktop should be working.

[Ren22]

My previous commits were added to this pull-request most probably because they were not merged previously

[LachlanStuart]

@Ren22 They would have been added because Github's "Squash and merge" ignores the commits in the PR branch and creates a single new commit on the master branch that contains a copy of all of the PR's changes. This means that if you start a 2nd branch off of the PR branch, your 2nd branch will include changes from the commits that overlap with the "Squash and merge" commit in master.

Sometimes this can make it hard to merge with master, so I recommend that whenever possible you start new branches off of master. If you have to make a branch off of an existing PR, once that PR is merged you should rebase from the old PR on to master.

Note that WebStorm/PyCharm's "Rebase" UI uses slightly different, nicer, terminology to Git's command line. With Git while in the PR2 branch you type rebase PR1 --onto master. With WebStorm in the Rebase dialog you "Rebase branch: PR2 onto: master from: PR1". The addition of that "from" word makes it a whole lot easier to reason about - cutting a slice of the commits from right after the "from" ref, up until the "branch" ref, and then pasting them after the "onto" ref.

[Ren22]

@LachlanStuart thank you for clear and simple explanations! What would be the best solution in this situation? Should I close this PR maybe , do a rebase --onto and then open a new one ?

[LachlanStuart]

For this PR I wouldn't worry. Now that you've merged master into this branch, there's no more potential pain from merging. The "Squash and merge" commit for this branch will include a few extra commit messages, but it's harmless.

[LachlanStuart]

When possible please prefer adding event handlers in the template, i.e. on line 10:

         <img :src="dataURI"
              :style="imageStyle"
              @click="onClick"
              @wheel="onWheel" <--- Add this
              ref="visibleImage"
              class="isotope-image"/>

If the reason you've done this is because of the IE bug, firstly please actually test it on IE because I'm 90% sure that Vue uses addEventListener under the hood instead of using onwheel-style attributes, and if it is the case then leave a comment.

[LachlanStuart]

• If none of these tests match, this function returns undefined. It should be structured so that it falls back to 'hold CTRL and scroll' if it doesn't recognize the OS.
• Calling getOS() so many times is wasteful - call it once, assign it to a variable and test the variable

[LachlanStuart]

"out" needs an "of" after it. Although ideally this wouldn't say "into/out of" at all, as English makes it an awkward phrase. Google Maps actually avoids it and says "Use ctrl + scroll wheel to zoom the map"

[LachlanStuart]

• Keep the HTML declarative by avoiding altering elements directly. Create data and computed properties as needed so that this element has e.g. :class="overlayClass" or :class="{fadeIn: overlayFadingIn, fadeout: overlayFadingOut}"
• If multiple events occur, these setTimeouts are going to overlap and fight with each other causing flickering or pulsating. To avoid this fighting, it's best to check for and cancel any previous timeouts that were caused by this event. setTimeout returns a timeoutID value that can be cancelled by clearTimeout.
• You only need one setTimeout here - see my comment below about doing fade in/out with only 2 classes
• Please keep consistency and don't put a line break in } else if

[LachlanStuart]

• In this project, all CSS is global. That means that any .fadeIn defined here would conflict with any .fadeIn defined in any other class. For this reason it's very important that we always pick unique names for CSS.
  The approach used elsewhere ranges from using BEM naming conventions, to just using a reasonably unique prefix (e.g. MetadataEditor uses the .md- prefix). I don't care too much which approach is used because I intend to move this project to scoped CSS at some point in the future. But for now, it's important to keep CSS names distinctive, because ambiguous names will only make future refactoring harder. If in doubt, use BEM.
  All 4 of these new classes need to be changed. As an example, they could be named this way using BEM: .image-loader__overlay, .image-loader__overlay--fade-in, .image-loader__overlay--fade-out, .image-loader__overlay-text
• You shouldn't need 3 classes for the fade-in/fade-out. With CSS transitions, the base class can be set up to handle the normal invisibility as well as the fade-out animation, and the modifier class should only need to handle the fade-in animation, e.g.:

.image-loader__overlay {
  pointer-events: none;
  background-color: #fff;
  width: 100%;
  height: 100%;
  position: absolute;
  opacity: 0;
  transition: 1.1s;
  z-index: 3;
}
.image-loader__overlay--visible {
 background-color: black;
 opacity: 0.6;
 transition: 0.7s;
}

[LachlanStuart]

:style="hideMessage" could be more simply written as v-show="scrollBlock"

[Ren22]

@LachlanStuart thank you again for thorough code review and suggestions!