-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Allow for functional links in amp-accordion headers #9335
Changes from 20 commits
3eec046
63e71b1
c60986b
d9a44e9
c44c305
c9aa814
db094ab
63f7716
fb29144
5ead669
f56b4ee
26be4ac
59f1dc3
c221184
319eaac
cc85994
fc772ec
47e7b6f
f90c133
30a469a
552c3f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,8 +93,7 @@ class AmpAccordion extends AMP.BaseElement { | |
header.setAttribute('tabindex', 0); | ||
} | ||
this.headers_.push(header); | ||
header.addEventListener('click', | ||
this.onHeaderPicked_.bind(this)); | ||
header.addEventListener('click', this.clickHandler_.bind(this)); | ||
header.addEventListener('keydown', this.keyDownHandler_.bind(this)); | ||
}); | ||
}); | ||
|
@@ -177,8 +176,22 @@ class AmpAccordion extends AMP.BaseElement { | |
} | ||
|
||
/** | ||
* Handles key presses on an accordion expand/collapse its content or | ||
* move focus to previous/next header. | ||
* Handles clicks on an accordion header to expand/collapse its content. | ||
*/ | ||
clickHandler_(event) { | ||
// Need to support clicks on any children of the header except | ||
// for on links, which should not have their default behavior | ||
// overidden. | ||
const target = dev().assertElement(event.target); | ||
if (target.tagName != 'A') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this may not work for image links ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
// Don't use clicks on links in header to expand/collapse. | ||
this.onHeaderPicked_(event); | ||
} | ||
} | ||
|
||
/** | ||
* Handles key presses on an accordion header to expand/collapse its content | ||
* or move focus to previous/next header. | ||
* @param {!Event} event keydown event. | ||
*/ | ||
keyDownHandler_(event) { | ||
|
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.
Should we support anything else here besides anchor tags?