Don't check event.shiftKey if event doesn't exist

[JacksonRG]

Fixing the shift-drag bug #4112 introduced a problem when dragging a clip onto the timeline.

The issue was evaluating an attribute of an event, when there wasn't an event.

[jonoomph]

LGTM!

[ferdnyc]

@jonoomph @JacksonRG

event is actually never going to not be undefined — it was never passed into the function. There's really no point in checking it at all. If moveBoundingBox is hoping to use event, it needs to be supplied by the caller.

(Which at times can tricky, because they don't always have it either. $scope.moveItem(), for example, doesn't get an event to pass. The drag methods of the clip and transition directives do, though — it's e, which would have to be added as an argument to moveBoundingBox. Or mBB() could just take a boolean argument indicating whether Shift is pressed, after first being checked by the caller. That way it doesn't have to pass the whole event object.)

[jonoomph]

Yeah, I also had the idea of passing in the shift key arg, so the callers can handle where they get that info. But, the typeof(event) !== "undefined" does in fact appear to be a valid way to handle this also... in my testing. It prevents the regression introduced by #4112, which must mean that event is undefined in the ManualMove case.

[jonoomph]

However, I do like the idea of passing in the shift key arg, as it's a bit more clear what is happening.

[JacksonRG]

Does sound like a cleaner solution. I admit this was a quick patch, and the type check prevents the method from crashing when we try to evaluate the event.shiftKey

[ferdnyc]

the type check prevents the method from crashing when we try to evaluate the event.shiftKey

the typeof(event) !== "undefined" does in fact appear to be a valid way to handle this also...

Oh, no, agreed — my only point was that event will always be undefined, with the current code. Checking it before trying to dereference it is definitely a good idea, but it'd kind of make more sense to just remove that block of code entirely, because as things currently stand there will never be a valid event defined inside that function.

If we want moveBoundingBox() to have access to the shift key state, we need to pass it in.