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

Allow the use of a class for the non highlighted #27

Merged
merged 4 commits into from
Jun 14, 2017

Conversation

danvc
Copy link
Contributor

@danvc danvc commented Jun 14, 2017

There is a issue that doesn’t allow the user use a specific class for
the elements that has no highlighted texts.

There is a issue that doesn’t allow the user use a specific class for
the elements that has no highlighted texts.
@bvaughn
Copy link
Owner

bvaughn commented Jun 14, 2017

className is already used for the top level <span className={className}>

@danvc
Copy link
Contributor Author

danvc commented Jun 14, 2017

Yes, but it should be applied to the children as well, don you think?

It's creating

<span class="my-parent-class">
  <span>xxx</span>
  <mark>y</mark>
  <span>zzz</span>
</span>

The main idea is that the user would not need to create extra classes, but, inherit it from the actual implementation.

@bvaughn
Copy link
Owner

bvaughn commented Jun 14, 2017

Yes, but it should be applied to the children as well, don you think?

That would limit what you're able to do with the class. 😕 Plus it wouldn't be a backwards-compatible change.

@danvc
Copy link
Contributor Author

danvc commented Jun 14, 2017

how about a new prop name?

Please, take a look in the new implementation.

@bvaughn
Copy link
Owner

bvaughn commented Jun 14, 2017

Hm. I guess I don't object, although I think inactive is probably better than nonActive

The props docs would also need to be updated.

@danvc
Copy link
Contributor Author

danvc commented Jun 14, 2017

I believe that now it's correct @bvaughn

@bvaughn bvaughn merged commit affaf92 into bvaughn:master Jun 14, 2017
@danvc
Copy link
Contributor Author

danvc commented Jun 14, 2017

Thank you so much Brian. You did a great job!

@bvaughn
Copy link
Owner

bvaughn commented Jun 14, 2017

Cool. Thanks for the contribution. 😄

I apologize, but last night when you opened this PR I was watching TV with my wife and I didn't give the property name we discussed enough thought. activeClassName isn't the property this is the opposite of. Actually it's the opposite of highlightClassName so this property should have been named something more like unhighlightClassName. I've gone ahead and renamed after merging your PR and updated the docs.

This feature is out as v0.8.0!

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

2 participants