These are chat archives for openseadragon/openseadragon

25th
Mar 2016
Antoine Vandecreme
@avandecreme
Mar 25 2016 14:12
@dwrogers For now overlays, are always div elements. I suppose that with the correct CSS you should already be able to make ellipses.
I don't think a closed path is possible in CSS though.
Grant Echols
@gechols
Mar 25 2016 15:23
Are overlays div elements by convention or by enforcement? In other words, what would happen if someone tried to make a 'paragraph' or other element into an overlay?
Antoine Vandecreme
@avandecreme
Mar 25 2016 15:48
if just an id is provided, OSD creates a div
you could probably pass another kind of element though.
Ian Gilman
@iangilman
Mar 25 2016 16:40
@gechols Absolutely, you can pass any kind of DOM element. Some may work better than others though
Antoine Vandecreme
@avandecreme
Mar 25 2016 17:05
I am looking into splitting the overlay class. Currently, you could transform a pointOverlay to a rectOverlay and vice-versa with the update method: https://github.com/openseadragon/openseadragon/blob/master/src/overlay.js#L275-L286
That won't be possible anymore. Is it an acceptable breaking change?
Ian Gilman
@iangilman
Mar 25 2016 17:06
Seems okay to me (assuming there are benefits to splitting the class). I suppose you could mention it in a ticket in GitHub to see if anyone raises their hand
Antoine Vandecreme
@avandecreme
Mar 25 2016 17:11
Maybe instead of splitting the class, we can just split the methods
I will try that approach first
Ian Gilman
@iangilman
Mar 25 2016 17:13
Cool
I don't think it's worth splitting "horizontal only" and "vertical only" into their own classes, so maybe if supporting those can be done with split methods that gets you a good deal of the way to a cleaner mechanism?
Antoine Vandecreme
@avandecreme
Mar 25 2016 18:34
Yeah I agree about "horizontal only" and "vertical only". It is more the current code that I find hard to read/debug.
So I am thinking to split drawHTML into _drawHTMLRectOverlay and _drawHTMLPointOverlay.
Ian Gilman
@iangilman
Mar 25 2016 18:58
Cool... I'm all for cleanup!
Antoine Vandecreme
@avandecreme
Mar 25 2016 20:58
I started a PR
Ian Gilman
@iangilman
Mar 25 2016 21:09
Cool :)
David Rogers
@dwrogers
Mar 25 2016 22:13
@avandecreme newbie question if you have time. I know what a PR is, but what does it mean to "start a PR"?
Antoine Vandecreme
@avandecreme
Mar 25 2016 23:02
I just opened a pull request so that any one can see my changes/ make any comments but it is not ready to be merged yet. See #896
David Rogers
@dwrogers
Mar 25 2016 23:53
@avandecreme Got it. Thank you!