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

Drop camelCase to kebab-case auto-conversion #62

Merged
merged 5 commits into from
Apr 3, 2020

Conversation

zamith
Copy link
Contributor

@zamith zamith commented Apr 1, 2020

Why:

  • There are css architectures that use uppercase class names in their
    naming convention, for example, SUITCSS (https://suitcss.github.io/).
    Therefore we don't want to restrict that.

This change addresses the need by:

  • Allowing binary or atom css classes to go through untouched, even in
    lists. When they're defined as a property we lose control over exactly
    how it's going to be named and that can also be improved, but not as
    big of a deal.

As a side note, I noticed that the project does not use mix format.
Any particular reason?

Why:

* There are css architectures that use uppercase class names in their
  naming convention, for example, SUITCSS (https://suitcss.github.io/).
  Therefore we don't want to restrict that.

This change addresses the need by:

* Allowing binary or atom css classes to go through untouched, even in
  lists. When their defined as a property we lose control over exactly
  how it's going to be named and that can also be improved, but not as
  big of a deal.

As a side note, I noticed that the project does not use `mix format`.
Any particular reason?
@msaraiva
Copy link
Member

msaraiva commented Apr 3, 2020

Hi @zamith!

Thanks for the PR.

The camel case to kebab case auto-conversion will be removed very soon (before the next alpha version) so I believe the changes in this PR will no longer be necessary (please, let me know if it still does). The whole discussion can be found here.

I noticed that the project does not use mix format. Any particular reason?

Yeah, I have mixed feelings about the formatter. I think some of the decisions it makes are really bad, especially regarding indentation. However, I understand the importance of a unified format for all developers so I will eventually start using now as we starting to get more contributions. So I'll make sure I'll run it before releasing the next alpha version.

Cheers.

Copy link
Member

@msaraiva msaraiva left a comment

Choose a reason for hiding this comment

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

@zamith with the suggested changes your PR is exactly what needs to be done to drop the camel-to-kebab and close #52! Please let me know if you disagree in any way.

lib/surface.ex Outdated Show resolved Hide resolved
lib/surface.ex Outdated Show resolved Hide resolved
zamith and others added 3 commits April 3, 2020 17:59
Co-Authored-By: Marlus Saraiva <marlus.saraiva@gmail.com>
Co-Authored-By: Marlus Saraiva <marlus.saraiva@gmail.com>
@zamith
Copy link
Contributor Author

zamith commented Apr 3, 2020

@msaraiva Done. Also removed to_kebab_case/1 as it's not used anymore

@msaraiva msaraiva changed the title Allow for full control over css classes in lists Drop camelCase to kebab-case auto-conversion Apr 3, 2020
@msaraiva msaraiva merged commit 71999b8 into surface-ui:master Apr 3, 2020
@zamith zamith deleted the zamith/uppercase-css-classes branch April 4, 2020 11:58
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

Successfully merging this pull request may close these issues.

2 participants