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

replace dedupe with distinct #105

Merged
merged 1 commit into from
Feb 14, 2016
Merged

replace dedupe with distinct #105

merged 1 commit into from
Feb 14, 2016

Conversation

tonsky
Copy link
Contributor

@tonsky tonsky commented Feb 14, 2016

dedupe is meant for sorted collections only

user=> (dedupe [1 2 2 3 2])
(1 2 3 2)
user=> (distinct [1 2 2 3 2])
(1 2 3)

r0man added a commit that referenced this pull request Feb 14, 2016
replace `dedupe` with `distinct`
@r0man r0man merged commit bca780e into r0man:master Feb 14, 2016
@r0man
Copy link
Owner

r0man commented Feb 14, 2016

Oh, thanks! :)

@nha
Copy link
Contributor

nha commented Feb 14, 2016

That was the original intent in my mind.

I am completely fuzzy on the details about how CSS operates, but I have a case where removing any of the two "aligned" classes of the same element results in visual changes there : http://semantic-ui.com/examples/login.html

@r0man
Copy link
Owner

r0man commented Feb 14, 2016

@nha ok, I found the follwoing 2 links. Looks like the W3C spec is loose and it is a known problem with Semantic Ui:

So, I guess I revert this change again? @tonsky Are you fine with that?

@tonsky
Copy link
Contributor Author

tonsky commented Feb 15, 2016

Oh my... Semantic UI decision to rely on exact string value of className in presence of classList browser API seems very very unwise. I guess it won’t harm much to remove deduplication though. But in that case we should get rid of dedupe as well.

@tonsky
Copy link
Contributor Author

tonsky commented Feb 15, 2016

BTW I checked with React, they use className, not classList, to change classes (because of IE compatibility), so duplicate classNames should go though React without obstacles

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.

None yet

3 participants