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

Inconsistencies and oddities in web TS #1356

Open
literat opened this issue Apr 22, 2024 · 0 comments
Open

Inconsistencies and oddities in web TS #1356

literat opened this issue Apr 22, 2024 · 0 comments

Comments

@literat
Copy link
Collaborator

literat commented Apr 22, 2024

I came across this while making the DS-449:

  1. Dropdown.ts refers to the visual CSS class DropdownWrapper, but should look for data-spirit-element
  • refactor without this class reference, probably best by structure
  1. It is not clear how the data configuration is handled: somewhere there is getOptions() (Dropdown), mostly just *.dataset directly
  • mostly we work directly, but you could somehow unify the approach
  1. configuration data attributes do not contain a spirit infix, e.g. data-max-file-size
  • we want to fix this within BC v1
  1. methods sometimes define an output type (: void etc.), sometimes not
  • typescript can usually infer itself, so rather remove
  1. some plugins have this.state, others either not at all (but could), or put individual properties directly into this
  • we don't want to treat plugins like React, we don't want to use this.state, but rather separate class properties
  1. events (resize, load, ...) are sometimes in constants, sometimes not
  • we want to have ideally in constants
  1. The packages/web/src/js/utils/{}tests{}/index.test.ts contains not only exports but also code
  • indexes contain only imports/exports and not code, move to files named accordingly
  1. js/utils usually don't have tests (or is this related to the previous point?)
  • add tests
  1. naming constants with class - CLASS_NAME, CLASSNAME, _CLASSNAME
  • I would go the way of CLASS_NAME_ prefixes like we have events
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

No branches or pull requests

1 participant