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

fix/multiple classnames #1074

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

Conversation

gaetan-hexadog
Copy link

@gaetan-hexadog gaetan-hexadog commented Nov 20, 2022

Description

In some cases, we want to be able to pass multiple class names like so 'list first second'. However, this is making an error because classList.add doesn't support space.

This PR attempts to implement allowing multi-class configs for all elements across the board by allowing to pass an array of strings for classNames.

It takes into account feedbacks from PR#907 submitted by @mikebronner

Fix #832 #889

Types of changes

  • Chore (tooling change or documentation change)
  • Refactor (non-breaking change which maintains existing functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • My code follows the code style of this project.
  • I have added new tests for the bug I fixed/the new feature I added.
  • I have modified existing tests for the bug I fixed/the new feature I added.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@gaetan-hexadog gaetan-hexadog changed the title fix/space in classnames fix/multiple classnames Nov 20, 2022
@mtriff
Copy link
Member

mtriff commented Nov 30, 2022

Thanks for this! Can you merge the latest version of master into this branch? After that I will review it.

@gaetan-hexadog
Copy link
Author

Thanks for this! Can you merge the latest version of master into this branch? After that I will review it.

Done ;-)

@ezitisitis
Copy link

I have a feeling that when @mtriff will get back to this, @gaetan-hexadog will need to merge latest master once again 🤣

@gaetan-hexadog
Copy link
Author

@mtriff let me know when you'll be able to merge this PR so I can merge the latest version of master before (don't want to do it if you're not going to merge it quickly) ;-)

@btmluiz
Copy link

btmluiz commented Apr 8, 2023

Hey, does anyone know when this PR will be merged?

@ammaraslam10
Copy link

+1

2 similar comments
@KarelBrijs
Copy link

+1

@J2-Tech
Copy link

J2-Tech commented Dec 15, 2023

+1

@otherprod
Copy link

Please come back, we need the multiple class override, thx

@MeetJF
Copy link

MeetJF commented Feb 4, 2024

We need this

@liamseys
Copy link

liamseys commented Apr 5, 2024

@gaetan-hexadog Whats with this?

@gaetan-hexadog
Copy link
Author

@gaetan-hexadog Whats with this?

No news from the repo'd owner. I won't work on it for the third time if I don't have the insurance it will be merged quickly.
Feel free to use my PR if you want.

@ammaraslam10
Copy link

There should be a protocol for when the owner stops responding for more than a year...

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.

Class names can't contain spaces
10 participants