Add support for start and end adornments #255
Conversation
|
This addresses #248 |
|
This may also address #241 because I noticed the same issue with what I was working on. YMMV since I wasn't working on that per se |
|
@me245 Thank you very much for your PR! 🎉 I'm pretty busy currently but I'll come back to it this weekend. |
|
"This weekend" is now. 🙈 |
There was a problem hiding this comment.
@me245 Thanks again for this great first PR! 🎉 It's been a while, but I really like it. Maybe you could just update the few things I commented?
A usage example of this feature as a new story (in the storybook) would be really useful, too. 👍
src/ChipInput.js
Outdated
| @@ -423,6 +479,7 @@ class ChipInput extends React.Component { | |||
| helperText, | |||
| id, | |||
| InputProps = {}, | |||
| inputAdornmentRender = defaultInputAdornmentRenderer, | |||
There was a problem hiding this comment.
This should be called renderer
| inputAdornmentRender = defaultInputAdornmentRenderer, | |
| inputAdornmentRenderer = defaultInputAdornmentRenderer, |
| @@ -575,6 +653,8 @@ ChipInput.propTypes = { | |||
| chipRenderer: PropTypes.func, | |||
| /** Whether the input value should be cleared if the `value` prop is changed. */ | |||
| clearInputValueOnChange: PropTypes.bool, | |||
| /** The color to render the chip based on the color palette **/ | |||
There was a problem hiding this comment.
The other new props, i.e. startAdornment, endAdornment and adornmentRenderer should also be added and documented here.
| <Chip | ||
| key={key} | ||
| className={className} | ||
| style={{ pointerEvents: isDisabled ? 'none' : undefined, backgroundColor: isFocused ? blue[300] : undefined }} | ||
| onClick={handleClick} | ||
| onDelete={handleDelete} | ||
| label={text} | ||
| color={color || 'default'} |
There was a problem hiding this comment.
The default value for color could be specified in the component's defaultProps so that it appears in the documentation, too.
| /> | ||
| ) | ||
|
|
||
| export const defaultInputAdornmentRenderer = (inputAdornment, additionalClasses) => { |
There was a problem hiding this comment.
Not using this function would probably break a lot of things. Is a prop for customizing the renderer actually needed ? 🤔
There was a problem hiding this comment.
The biggest item for this would be to resolve the classes for styling. It was loosely based on what was happening from any of the inputs in Material-ui compared to BaseInput.
When you say not using this function, can you clarify what issue you are seeing? It is a default renderer function and if someone were to override it, I would be expecting them to resolve the styling issues in their manual implementation anyhow. I was opting for customization capability more than anything.
There was a problem hiding this comment.
But I could easily just hide this from being a prop if that is what you are going after. Let me know if you want me to do that instead
|
I added some story book stories and noticed some glaring style issues and needed to fix those. Let me know if there is something else that you want me to take a look at |
|
@me245 Sorry, I've been pretty busy (and also I don't get Slack notifications for new comments anymore. Thank you for the new stories. I'll review this this weekend and probably merge the PR. Thank you very much! 🎉 |
|
@leMaik any chances for this to get merged? :) |
|
@viart Yes. As soon as either
If you want to give this a shot, I'd really appreciate a succeeding PR! (You can fork the fork 😉) Cc @geilecj |
|
Any chance this is getting merged, or I could have a shot fixing it and merging it, if that's ok, I'd need a few pointers due to my newness to OpenSource |
Initial work was designed for supporting different input variants (IE. outlined and filled inputs). Mostly style changes, however there was some items being overwritten on alternative variants of inputs. Due to those differences, code was changed to make the rendering more uniform across different input styles
Also adds color to the chip renderer by default. Should not be an breaking API change as it includes the same default value and is within an param object.
Allows for some more style overriding for chips, and for start/end adornments