Skip to content

Fix linting error (#775) #779

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

Merged
merged 4 commits into from
Nov 8, 2021

Conversation

rajakvk
Copy link
Contributor

@rajakvk rajakvk commented Oct 22, 2021

Closes #775 Item 5
File: app/components/ember-anchor.js

@rajakvk rajakvk mentioned this pull request Oct 22, 2021
Copy link
Contributor

@locks locks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to confirm that the updated code keeps the same behavior!

@@ -8,7 +6,7 @@ export default class EmberAnchor extends AnchorComponent {
// This overrides Ember Anchor to support scrolling within a fixed position element
_scrollToElemPosition() {
let qp = this.anchorQueryParam;
let qpVal = this.get(get(this, 'attrs.a') ? 'a' : `controller.${qp}`);
let qpVal = this[this['attrs.a'] ? 'a' : `controller.${qp}`];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe what the original code was doing is this:

Suggested change
let qpVal = this[this['attrs.a'] ? 'a' : `controller.${qp}`];
let qpVal = this.a ?? this.controller[qp];

this['attrs.a'] is actually subtly wrong because it will try to retrieve the property named attrs.a.
Ember's get doesn't allow properties with periods, because it retrieves nested properties.

So, this.get('attrs.a') translated to this.attrs.a and this['attrs.a'] translates to this['attrs.a'].
Or, represented as objects:

{
  attrs: { a: "" }
}

{
  "attrs.a": {}
}

@rajakvk rajakvk force-pushed the rk-775-fix-linting-error-4 branch from d0ef851 to 9a7b385 Compare November 5, 2021 07:28
@locks locks force-pushed the rk-775-fix-linting-error-4 branch 2 times, most recently from 50be96e to 265a2e8 Compare November 8, 2021 02:13
@locks locks force-pushed the rk-775-fix-linting-error-4 branch from 265a2e8 to a2a8cae Compare November 8, 2021 17:39
@locks locks merged commit 9d5f38d into ember-learn:master Nov 8, 2021
@jenweber jenweber temporarily deployed to ember-api-docs-staging January 10, 2022 14:50 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix eslint issues
3 participants