component-library icon indicating copy to clipboard operation
component-library copied to clipboard

Update tabs.js

Open aaronware opened this issue 3 years ago • 0 comments

Description of the Change

This is a minor change to handle targeting the click event in the case where you have nested elements within your tab links.

Example of the HTML markup that this impacts: Notice the nested span within the tab link markup

<a role="tab" id="tab-products" aria-controls="products" class="products-search-tab" aria-selected="true">
							Products <span class="count">(13)</span>
						</a>

When utilizing .addEventListener with a click event:

  • event.target refers to the element that triggered the event (i.e. the element the user clicked on)
  • event.currentTarget refers to the element that the event listener is attached to.

This is very important in this case because when you click on the span, the tab target content will actually be null by utilizing event.currentTarget instead, you then negate any child elements of the tab click event

Alternate Designs

I did not research any other alternatives as this is pretty standard

Benefits

Backward compatible with the current implementation and allows for a more complex HTML structure without breaking tab functionality

Possible Drawbacks

None that I came across

Verification Process

  • Tested on major browsers as of this Pull request creation
  • Testing is easy to replicate. Create a tab link with an element within it <span> are pretty realistic, click on the span with the current code base, you will see it break will result in no tab showing. By changing the event you will see that even clicking on the span the event will bubble properly

Checklist:

  • [ X] My code follows the code style of this project.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [X ] All new and existing tests passed.

aaronware avatar Jul 17 '22 16:07 aaronware