search

recogito / text-annotator-js

A JavaScript library for text annotation.
BSD 3-Clause "New" or "Revised" License
22 stars 7 forks source link

Reverted breaking API change, removal of the `denormalizeRectWithOffset` #182

Open oleksandr-danylchenko opened 5 days ago

oleksandr-danylchenko commented 5 days ago

Changes Made

Reverted the https://github.com/recogito/text-annotator-js/commit/945c09db538bf6bedfe4740be14bdeb872b275bf commit, where the normalizeRects.ts utility file was deleted. However, its utilities are used by the consumers that implement custom popup components.

rsimon commented 5 days ago

Sorry, in this case I have to contradict. I'm fine if we find sensible names for it. But this way, when I looked at those methods for a second time, I had already forgotten what direction "normalize" vs. "denormalize" goes. Having two args of the same type doesn't help either. We're really talking about a single line of code here, and it's pretty obvious what it does if you see it. But with just generic "(de)normalize" as name, readability IMO suffers massively, for a single line of (readable) avoided code.

oleksandr-danylchenko commented 5 days ago

Hmm, that's reasonable. Maybe I overprojected my familiarity with the "normalize/denormalize" functions.

Would smth like the "toParentBounds" & "toViewportBounds" sound clearer to you?