Skip to content

amp-bind: verify warning improvements#9103

Merged
kmh287 merged 3 commits intoampproject:masterfrom
kmh287:bind_verify_warning_improvements
May 9, 2017
Merged

amp-bind: verify warning improvements#9103
kmh287 merged 3 commits intoampproject:masterfrom
kmh287:bind_verify_warning_improvements

Conversation

@kmh287
Copy link
Copy Markdown
Contributor

@kmh287 kmh287 commented May 2, 2017

When #development=1, we shouldn't warn users that bind attributes don't match their non-bind counterparts when non-bind counterparts aren't valid AMP attributes. For instance, amp-carousel supports the [slide] bind attribute, but does not support a non-bind slide attribute.

Fixes #8933

/to @choumx

@kmh287
Copy link
Copy Markdown
Contributor Author

kmh287 commented May 9, 2017

@choumx


// Don't show a warning for bind-only attributes,
// like 'slide' on amp-carousel.
const bindOnlyAttrs = BIND_ONLY_ATTRIBUTES[element.tagName.toLowerCase()];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can avoid toLowerCase by uppercasing the map keys.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

*/
let BoundElementDef;

/** @private {!Object<string, !Array<string>>} */
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you add a short description for this map?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@kmh287 kmh287 force-pushed the bind_verify_warning_improvements branch from 1bd0ce4 to f140f64 Compare May 9, 2017 16:55
@kmh287 kmh287 merged commit 07f288a into ampproject:master May 9, 2017
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.

4 participants