svg.js icon indicating copy to clipboard operation
svg.js copied to clipboard

Fix throwing when call svg.ungroup()

Open OpenGG opened this issue 3 years ago • 5 comments

Current status: svg.ungroup() throws with confusing message "Uncaught TypeError: parent.screenCTM is not a function". Fix: Early throws with determine message.

OpenGG avatar Nov 16 '21 11:11 OpenGG

Another possibility would be to just call flatten instead. But that might be confusing. We really rarely throw errors in the lib because validating all user input would quickly bloat the code. How did you discover this?

Fuzzyma avatar Nov 16 '21 11:11 Fuzzyma

@Fuzzyma Am really just newbee digging around. If ungroup() not supposed to be called on svg root, maybe it should be moved to Group class?

OpenGG avatar Nov 18 '21 11:11 OpenGG

Well you can call it on a nested svg. Also there are other container elements that you can ungroup. Basically everything that can have children (defs for example)

Fuzzyma avatar Nov 18 '21 12:11 Fuzzyma

@Fuzzyma Then what should be done with this issue in your opinion? Option 1: .ungroup() can be call with "basically everything" In this case, early throwing or polymorphism? Both requires special handling with different elements. Option 2: Limit .ungroup() to g Relatively simple, may break flatten(). Option 3: Do nothing Calling .ungroup() with svg cause throwing, other elements may too.

OpenGG avatar Nov 24 '21 03:11 OpenGG

Ungroup can ONLY be called for container elements. The only option is leave it or implement your fix. However your fix will break for other elements than svg because they don't have the isRoot method. So I guess we leave it for now

Fuzzyma avatar Nov 24 '21 08:11 Fuzzyma