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

Docs/improve class names docs #585

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

mbohal
Copy link
Contributor

@mbohal mbohal commented Jan 30, 2025

No description provided.

This commit creates a new folder `src/jsHelpers` for `classNames` and `transferProps`
functions.

The aim is to have the code organization symetric to the
`src/components` folder for better developer orientation.
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jan 30, 2025
@mbohal mbohal requested a review from atmelmicro January 30, 2025 11:01
@bedrich-schindler
Copy link
Contributor

I disagree with such change.

  1. Documentation is strictly scoped to /docs dir. It's kind of standard among open-source software in Javascript word and Github as well (typically, /docs is served as Github Page). As it uses markdown, rendering engine can be easily switched to something else, easily deployed and completely disconnected from code. Maintaining documentation structure make it easy to contribute, maintain, deploy and use by different rendering engines.
  2. Standard name should be js-helpers, not jsHelpers`. This is strongly connected to point 1.
  3. For some reason, we use utils in the code and JS helpers in the docs. To be honest, I have always been for utils/utilities as it is common name for such functions among UI libraries. In CSS and JS. We use helpers in our personal projects. I do not like calling it jsHelpers in the code. Everything in the code is JS, it does not make sense to prefix just this. If we want to stick to helpers, call it just helpers.
  4. I don't like changed documentation for classnames. We always spend time to have our examples pleasant for your eyes. This weird padding spikes my OCD. I get what you trying to achieve, but you just repeat same type of condition over and over and it does not display real life example which we try to demonstrate (?: and || / &&). So we have bad looking example with unreal example just to demonstrate few possible truthy/falsy examples, but in real life there are more not visible. If we do such exhaustive demonstration in all cases, examples would be enormous. Let's keep it simple. I

@bedrich-schindler
Copy link
Contributor

1+2. Let's do in in Martin's way.
3. Lets call it src/helpers
4. Let's split it in two examples. One of them is simpler than original and the second one demonstrates usage in MDN-like way, e.g. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/round

@mbohal
Copy link
Contributor Author

mbohal commented Feb 4, 2025

@bedrich-schindler it is fixed I think. Please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants