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

IDs and CSS classes have more than one purpose. #827

Open
meodai opened this issue Aug 22, 2022 · 2 comments
Open

IDs and CSS classes have more than one purpose. #827

meodai opened this issue Aug 22, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@meodai
Copy link
Contributor

meodai commented Aug 22, 2022

While touching lots of parts of the DOM i broke quite a few things.

Ex. the class highlightBox has some style on it but on the same time is used in the e2e tests. Changing the class caused test to fail. The situation is similar with some ID's (they are used to style but also to bind some behaviour to it). When changing anything that I thought would only be design related I would break things I did not expect to break.

My proposal would be to separate CSS / JS and testing selector logic. A possible implementation could be:

  • Only use .some-css-class to style things.
  • Use data-attributes to select things in js: data-somefancyfunction => document.querySelector('[data-somefancyfunction]') (Sometimes people like to use a class prefixed with js- rather than a data attribute, I like the data attribute because it can carry a value easily)
  • Only use data-e2etest="nameofthetest" to select things within the tests. /* and maybe remove those for production */

I think it would make it easier to do changes with no side-effects. It would be clear, what does what and by that made it easier to contribute to the code in the future.

What do you think?
Any thought on the separation of concerns itself and on my proposed implementation?

@meodai meodai added the enhancement New feature or request label Aug 22, 2022
@peterpeterparker
Copy link
Member

In NNS-dapp we use [data-tid="something"] attributes for test (jest and e2e) purpose. I'm not a fan of bloating the DOM with unmeaningful attributes but, these turned out to be really convenient.

@meodai
Copy link
Contributor Author

meodai commented Aug 29, 2022

cool I think we should do something similar. I keep running into issues because of that.

@nmattia nmattia changed the title ID's and CSS classes have more than one purpose. IDs and CSS classes have more than one purpose. Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants