geometry #20
geometry #20
Conversation
| * are positive, or smaller dimensions if they are negative. | ||
| */ | ||
| function adjustBox(box: ClientRect, top: number, right: number, bottom: number, left: number): ClientRect { | ||
| return { |
There was a problem hiding this comment.
Should we be checking to make sure the width and height are non-negative, and that the right/bottom is >= left/top ?
There was a problem hiding this comment.
This internal function is only used for the get functions presently, and is always passed a result from getBoundingClientRect, so I don't think there's anything to validate here.
This point may be valid for the width/height in the set functions - Dojo 1 ignored them entirely if they were negative. Meanwhile, glancing at the Dojo 1 code, I'm not actually sure if/how it was ignoring width/height if they were undefined (which we are doing below), even though it documents them as optional.
|
refs #8 |
| if (borderBox) { | ||
| height = rect.height; | ||
| } | ||
| else { |
There was a problem hiding this comment.
This can be shortened to:
let height = rect.height;
if (!borderBox) {
height -= ...
}(and same for width)
|
I can appreciate the focus on readability in this initial iteration, and I found myself debating whether some of the shortenings I suggested detract considerably from that. However, I think at the very least, shortening the code in It might be possible to shorten this more considerably, but I'd need to give my brain another iteration to catch that, and I suspect it'd involve some ugly |
| return { | ||
| 'box-sizing: border-box': testBoxMethod('setBorderBox', 'border-box', expected), | ||
| 'box-sizing: content-box': testBoxMethod('setBorderBox', 'content-box', expected) | ||
| } |
There was a problem hiding this comment.
Missing semicolon (same with the other setter tests)
This PR adds setters to @kfranqueiro's initial geometry getter work, along with tests. The actual setters could probably be refactored, but focus was put on readability.