Fix: Load annotator with the correct initial scale

[Minh-Ng]

Annotator is currently always being loaded with scale 1. When the true scale is set, the annotations are not re-rendered. As a result, annotations on page load will always be the same size.

Proposed changes:

• annotator.setScale() returns this (annotatorInstance) so that calls can be used in a builder format. ie annotator.setScale().init(), unfortunately prettier formats these calls all on one line. Might be useful to anyone who has forked the repository.
• Update the viewer scale just before loading the annotations
• Assign annotatedElement in Annotator constructor rather than init() so that annotatorInstance.setScale(trueScale) can be called before annotator.init(). Init() will render the annotations and require a rescale. As init() appears to be called right after constructing Annotator instances, I do not see an issue with this. In addition, child classes will be able to use this.annotatedElement.

[pramodsum]

Add a check in that one spot and lgtm!

[pramodsum]

Maybe throw an error here if a developer forgets to override getAnnotatotedEl() in their viewer specific annotator

[JustinHoldstock]

I understand the benefits of chaining, but since we already have access to the annotator instance, what is the benefit here over just calling instance.whatever() after calling instance.setScale()?

My thought process is that it goes against any coding paradigms we currently follow, in Preview

[Minh-Ng]

Anytime you can chain you will have access to the instance. Its just for readability so you can chain calls in a clear readable fashion.

NumberArray.setFirstFiveElements(5)
           .setMultiplier(3)
           .transform()
           .collect()

As opposed to

NumberArray.setFirstFiveElements(5)
NumberArray.setMultiplier(3)
NumberArray.transform()
NumberArray.collect()

Very minute semantic idea, although I understand it is probably not a good idea to throw it into the mix now. Essentially your functions will build properties of your instance and a few getter methods will actually display the transformed properties.

[tonyjin]

When in Rome :) Good discussion around patterns here but since we don't utilize that sort of chaining in Preview, please add it in a feature change.

[JustinHoldstock]

I know that's not how it works now, but wouldn't it make more sense if set scale set this.scale as well as set the attribute on the annotated element?

If so, then move to

const { scale } = event.scale;
//...
this.annotator.setScale(scale);

and setScale() can be

setScale(scale = 1) { 
  this.scale = scale;
  // ... set attribute and stuff
}

[Minh-Ng]

I originally misread what you meant.

Problem here is that setScale is in Annotator and the eventListener is in BaseViewer. Annotator does not have a Annotator.scale value. It gets its scale from data-scale. In addition, BaseViewer.initAnnotations() both sets up the scale listener and the annotator. At this point the Annotator needs to know the scale value which cannot be known through any method other than the 'load' event.

[Minh-Ng]

We can make this more clear by doing

if(event && event.scale) {
        this.scale = event.scale;
}

since this.scale is set to 1 by the property default.

[Minh-Ng]

Decided it was not good to rely on an abstract method in the constructor. Instead the initial scale will be passed in as an option for the init() method.