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

[PropTypes] Allow React components for matchElement #55

Closed
kachkaev opened this issue Aug 17, 2018 · 3 comments · Fixed by #56
Closed

[PropTypes] Allow React components for matchElement #55

kachkaev opened this issue Aug 17, 2018 · 3 comments · Fixed by #56

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Aug 17, 2018

Hi @helior and many thanks for your library! With react-highlighter I got from zero to all done in just 10 minutes, which is mind-blowing even in the React world!

I'm writing to share a minor issue I've faced when using <Highlight /> together with styled-components. Here's my code:

import styled from "styled-components";

const HighlightMark = styled.span`
  background: rgba(255, 255, 0, 0.3);
  border-radius: 4px;
  display: inline-block;
  margin: auto -3px;
  padding: 0 3px;
`;

// ... 

<Highlight search={highlightRegexp || false} matchElement={HighlightMark}>
  {text}
</Highlight>

This produces a warning in development environment:

Warning: Failed prop type: Invalid prop `matchElement` of type `function` supplied to `Highlighter`, expected `string`.

It'd be great if matchElement in PropTypes allowed using custom react components in addition to strings. The issue is only with the warning – all working great apart from that 🎉

@kachkaev
Copy link
Contributor Author

@helior would you be happy to release a new patch version if I submit a PR?

@helior
Copy link
Owner

helior commented Aug 23, 2018

Hi @kachkaev, I'm glad to hear you were up in running so quickly 👍 . This is a great idea to define components for the matching element. If you have a PR, I would be happy to release a version with the new feature.

@kachkaev
Copy link
Contributor Author

kachkaev commented Aug 23, 2018

Here you go: #56 🙌

Looking forward for a new patch release (I don't think we are adding a feature) 😉

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 a pull request may close this issue.

2 participants