New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
♻️Minor refactor for amp-orientation-observer
#24812
Conversation
|
||
/** @private {Array} */ | ||
this.gammaSmoothingPoints_ = []; | ||
/** @private {{beta: Array, alpha: Array, gamma: Array}} */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!Array<number>
gamma: DEFAULT_REST_VALUES.gamma, | ||
}; | ||
|
||
/** @private {{beta: Array, alpha: Array, gamma: Array}} */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!Array<number>
this.betaRange_ = this.parseAttributes_('beta-range', this.betaRange_); | ||
this.gammaRange_ = this.parseAttributes_('gamma-range', this.gammaRange_); | ||
AXES.forEach(axis => { | ||
this.range_[axis] = this.parseAttributes_( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will work with property renaming, since the properties on this.range_
will be renamed to things like a
and b
.
To get around this, you could do something like:
/** @private {!Object<string, !Array<number>>} */
this.range_ = {
alpha: [0, 360],
};
Alternatively, you can do this using arrays, and use array indicies from an enum (and move the axis names into an array as well).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed using string literals, also added cloning the default value. But let me know if you think using arrays is better. I just thought it might make the code slightly less readable.
Refactors repetitive methods and fixes a minor bug