Add testing identifier to elements

[lethemanh]

Add testing identifier to elements

How Has This Been Tested?

The test id class names have prefix testid:

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added the Signed-off-by statement at the end of my commit message.

[ericlinagora]

this is great, semantically what i would have done also. But please check because using css selectors makes it easy to find by hierarchy, if there is something else in browser to select, then maybe the browser part should be on it and only document-row here. We can then select just as easily .browser .document-row and if there's other stuff in the browser, we can reuse that parent class

[lethemanh]

I understand your idea, but this name has a rule behind, it is combination of component name and element name. Also, it helps to reduce the number of parent classes needed for selector

[ericlinagora]

But we don't want to reduce the number of parent selectors, and this means:

1. we have to repeat multiple times the parent's name
2. it's not composable if this is reused in multiple places
3. it the parent is repeated then using the hierarchy ensures we get the consistent set of children
4. if we don't need the parent in the selector because this isn't reused, repeated, etc ; then there's still no reason to repeat the parent selector on each child

please don't mix parent and child element, the DOM will let us choose how much hierarchy we want to specify and change it easily without changing the code if we need to

[ericlinagora]

Here is one example to clarify why this is a very bad idea, beyond just having to repeat code, being un-composable, breaching concern separation etc. Imagine we want to read each person in the list:

<li id="person">
  <div id="name">name 1</div>
  <div id="phone">123</div>
</li>
<li id="person">
  <div id="name">name 2</div>
  <div id="phone">123</div>
</li>

versus

<li>
  <div id="person-name">name 1</div>
  <div id="person-phone">123</div>
</li>
<li>
  <div id="person-name">name 2</div>
  <div id="person-phone">123</div>
</li>

see how much more complicated it would be to figure out which phone number goes with which name ? youd end up walking up and back down the tree and be specific to the html structure.

now imagine we want to reuse the name+phone for companies in another list.

now imagine we want to rename "person" into "employee" you have to change it at multiple places instead of one.

[ericlinagora]

because of the size of this PR, and the rapidly increasing risk of conflict, let's merge this for now

[ericlinagora]

Note - at this timestamp just closed the review because the code was updated while it was pending, so I'll restart instead of continuing on the old version

[ericlinagora]

overall this is a ton of work well done, my issue is that it was too early/quickly done, and i see testids more as a last resort, only better than using labels/roles or data-test-id. If there's any other reliable way to identify something we should probably do that. I think after the mini fixes we'll take it in, then do a second pass review with QA with actual script test cases etc.

[ericlinagora]

Approval to figure this out without the pending huge PR merge since it's benign