Skip to content
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

✨ [RUMF-385] improve click action naming #406

Merged
merged 5 commits into from
Jun 2, 2020

Conversation

BenoitZugmeyer
Copy link
Member

Motivation

Improve the name of click actions.

Changes

  • Improve the name extraction from elements by adding different strategies and adjusting priorities.
  • Don't collect actions if a name cannot be found.

Testing

This should be left on staging for a little while to verify that it does not break thing, and indeed improve the action names.


I have gone over the contributing documentation.

This commit improves the algorithm in various ways:

* Re-prioritize the name strategy:
  * use textual content in very precise cases in priority (ex: BUTTON
    element), together with other techniques (attributes, label, ...)
  * then fallback on the textual content of any element
* Use a related LABEL element
* Use a child OPTION to name a SELECT element
* Support more attributes: name, aria-labelledby, role=button
* Implement a parent recursion with a hard limit to 10 and up to a FORM element
* Ignore inline SCRIPT content and other hidden elements when computing textual content
* Ignore textual data when using content-editable elements and children
* Normalize the whitespaces
* Ignore BODY, HTML and HEAD elements
* getElementContent -> getActionNameFromElement
* element content -> element textual content
* other 'content' uses -> name
@BenoitZugmeyer BenoitZugmeyer requested a review from a team as a code owner May 28, 2020 09:48
@BenoitZugmeyer BenoitZugmeyer changed the title Benoit/improve click action naming ✨ [RUM] improve click action naming May 28, 2020
@BenoitZugmeyer BenoitZugmeyer changed the title ✨ [RUM] improve click action naming ✨ [RUMF-385] improve click action naming May 28, 2020
packages/rum/src/getActionNameFromElement.ts Outdated Show resolved Hide resolved
packages/rum/src/getActionNameFromElement.ts Outdated Show resolved Hide resolved
packages/rum/src/getActionNameFromElement.ts Outdated Show resolved Hide resolved
packages/rum/src/getActionNameFromElement.ts Outdated Show resolved Hide resolved
packages/rum/src/getActionNameFromElement.ts Show resolved Hide resolved
packages/rum/src/getActionNameFromElement.ts Outdated Show resolved Hide resolved
packages/rum/src/getActionNameFromElement.ts Show resolved Hide resolved
packages/rum/src/getActionNameFromElement.ts Outdated Show resolved Hide resolved
packages/rum/src/getActionNameFromElement.ts Outdated Show resolved Hide resolved
Comment on lines 131 to 132
// tslint:disable-next-line: prefer-for-of
for (let i = 0; i < elementsToRemove.length; i += 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

according to MDN for-of should work correctly on node list, typing does not seem to allow that though.
it could worth to check if it is supported by newer ts version but not important now

Copy link
Member Author

@BenoitZugmeyer BenoitZugmeyer May 28, 2020

Choose a reason for hiding this comment

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

It appears to be working on two conditions:

  • use the DOM.Iterable lib
  • and use ES6+ as a target or the downlevelIteration option

I'll keep the traditional for loop here for now.

* add some comments
* remove some comments
* rename some variables
* add an explicit constant for the recursion limit
@BenoitZugmeyer BenoitZugmeyer merged commit cbeea7b into master Jun 2, 2020
@BenoitZugmeyer BenoitZugmeyer deleted the benoit/improve-click-action-naming branch June 2, 2020 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants