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

Merged
merged 2 commits into from
Mar 4, 2025
Merged

Conversation

mbohal
Copy link
Contributor

@mbohal mbohal commented Jan 30, 2025

No description provided.

@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.

@mbohal mbohal added the BC Breaking change label Feb 5, 2025
Copy link
Collaborator

@lukasbriza lukasbriza left a comment

Choose a reason for hiding this comment

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

Add MDN-style documentation for the className function.

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.
@mbohal mbohal force-pushed the docs/improve-classNames-docs branch 2 times, most recently from 0f0491c to 8de1ffb Compare March 4, 2025 12:23
@mbohal mbohal force-pushed the docs/improve-classNames-docs branch from 8de1ffb to 83ef999 Compare March 4, 2025 12:29
@mbohal mbohal merged commit c2b0b44 into master Mar 4, 2025
10 checks passed
@mbohal mbohal deleted the docs/improve-classNames-docs branch March 4, 2025 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Breaking change documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants