Skip to content

Commit

Permalink
Cleaner code.
Browse files Browse the repository at this point in the history
  • Loading branch information
William Chou committed Jun 7, 2019
1 parent 19f3070 commit fb0dbe4
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 34 deletions.
9 changes: 1 addition & 8 deletions src/service/viewport/viewport-binding-def.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {Services} from '../../services';
import {computedStyle} from '../../style';
import {isExperimentOn} from '../../experiments';

Expand Down Expand Up @@ -209,18 +208,12 @@ export class ViewportBindingDef {
* Returns the margin-bottom of the last child of `element` with non-zero
* height, if any. Otherwise, returns 0.
*
* As of Safari 12.1.1, the getBoundingClientRect().height does not include the
* bottom margin of children and there's no other API that does.
*
* @param {!Window} win
* @param {!Element} element
* @return {number}
*/
export function marginBottomOfLastChild(win, element) {
if (
!isExperimentOn(win, 'bottom-margin-in-content-height') ||
!Services.platformFor(win).isSafari()
) {
if (!isExperimentOn(win, 'margin-bottom-in-content-height')) {
return 0;
}
let n = element.lastElementChild;
Expand Down
24 changes: 15 additions & 9 deletions src/service/viewport/viewport-binding-ios-embed-sd.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,20 +406,26 @@ export class ViewportBindingIosEmbedShadowRoot_ {
const content = this.wrapper_;
const rect = content./*OK*/ getBoundingClientRect();

// The Y-position of any element can be offset by the vertical margin
// The Y-position of `content` can be offset by the vertical margin
// of its first child, and this is _not_ accounted for in `rect.height`.
// This "top gap" causes smaller than expected contentHeight, so calculate
// and add it manually. Note that the "top gap" includes any padding-top
// on ancestor elements and the scroller's border-top.
const topGapPlusPaddingAndBorder = rect.top + this.getScrollTop();
const bottomGap = marginBottomOfLastChild(this.win, this.win.document.body);
// This causes smaller than expected content height, so add it manually.
// Note this "top" value already includes padding-top of ancestor elements
// and getBorderTop().
const top = rect.top + this.getScrollTop();

// As of Safari 12.1.1, the getBoundingClientRect().height does not include
// the bottom margin of children and there's no other API that does.
const childMarginBottom = marginBottomOfLastChild(
this.win,
this.win.document.body
);

const style = computedStyle(this.win, content);
return (
rect.height +
topGapPlusPaddingAndBorder +
top +
parseInt(style.marginTop, 10) +
bottomGap +
rect.height +
childMarginBottom +
parseInt(style.marginBottom, 10)
);
}
Expand Down
17 changes: 10 additions & 7 deletions src/service/viewport/viewport-binding-ios-embed-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,17 +253,20 @@ export class ViewportBindingIosEmbedWrapper_ {
const content = this.win.document.body;
const {height} = content./*OK*/ getBoundingClientRect();

// Unlike viewport-binding-natural.js, there's no need to calculate the
// "top gap" since the wrapped body accounts for the top margin of children.
// However, the parent's paddingTop and "bottom gap" must be added...
const bottomGap = marginBottomOfLastChild(this.win, content);
// Unlike other viewport bindings, there's no need to include the
// rect top since the wrapped body accounts for the top margin of children.
// However, the parent's padding-top (this.paddingTop_) must be added.

// As of Safari 12.1.1, the getBoundingClientRect().height does not include
// the bottom margin of children and there's no other API that does.
const childMarginBottom = marginBottomOfLastChild(this.win, content);

const style = computedStyle(this.win, content);
return (
height +
this.paddingTop_ +
parseInt(style.marginTop, 10) +
bottomGap +
this.paddingTop_ +
height +
childMarginBottom +
parseInt(style.marginBottom, 10)
);
}
Expand Down
27 changes: 17 additions & 10 deletions src/service/viewport/viewport-binding-natural.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,19 +210,26 @@ export class ViewportBindingNatural_ {
// there's no workable analog for either ios-embed-* modes.
const content = this.getScrollingElement();
const rect = content./*OK*/ getBoundingClientRect();
const style = computedStyle(this.win, content);
// The Y-position of any element can be offset by the vertical margin

// The Y-position of `content` can be offset by the vertical margin
// of its first child, and this is _not_ accounted for in `rect.height`.
// This "top gap" causes smaller than expected contentHeight, so calculate
// and add it manually. Note that the "top gap" includes any padding-top
// on ancestor elements, and the "bottom gap" remains unaddressed.
const topGapPlusPadding = rect.top + this.getScrollTop();
const bottomGap = marginBottomOfLastChild(this.win, content);
// This causes smaller than expected content height, so add it manually.
// Note this "top" value already includes padding-top of ancestor elements
// and getBorderTop().
const top = rect.top + this.getScrollTop();

// As of Safari 12.1.1, the getBoundingClientRect().height does not include
// the bottom margin of children and there's no other API that does.
const childMarginBottom = Services.platformFor(this.win).isSafari()
? marginBottomOfLastChild(this.win, content)
: 0;

const style = computedStyle(this.win, content);
return (
rect.height +
topGapPlusPadding +
top +
parseInt(style.marginTop, 10) +
bottomGap +
rect.height +
childMarginBottom +
parseInt(style.marginBottom, 10)
);
}
Expand Down

0 comments on commit fb0dbe4

Please sign in to comment.