Add inputClasses option
#602
Conversation
src/autocomplete.js
Outdated
| @@ -471,6 +473,11 @@ export default class Autocomplete extends Component { | |||
| } | |||
| } | |||
|
|
|||
| let inputClassesFinal = '' | |||
| if (inputClasses && (typeof inputClasses === 'string' || inputClasses instanceof String) && inputClasses.length > 0) { | |||
There was a problem hiding this comment.
@andreyyudin Thanks for adding this new option. Our other string options don't go through as thorough a check, so we'd be happy to leave this as if (inputClasses) for simplicity and consistency of treatment with the rest of the string. Which situations would the extra conditions prevent (if an [object Object], 1 or true slips in, it wouldn't be the end of the world)?
There was a problem hiding this comment.
Thank you @romaricpascal, I am happy if it is as you suggest. I think I was just being overly careful.
There was a problem hiding this comment.
Brill! Would you prefer I make the change (I'll likely have to force push on your branch to keep the rebuild the last commit) or are you OK doing it?
There was a problem hiding this comment.
If that is not too much trouble to do the change on your end, please go ahead, otherwise I am happy to do it too, thank you.
There was a problem hiding this comment.
No problem at all, I'll make the change, rebase your branch and re-build dist to get everything ready for merging 😊
There was a problem hiding this comment.
@andreyyudin That will likely involve a force push to your branch, as a heads up 😊
There was a problem hiding this comment.
Which GitHub is not letting me do, so I'll open a new PR with your rebased commits and the rebuild to facilitate the merge 😊
romaricpascal
force-pushed
the
feature/inputClasses
branch
3 times, most recently
from
ca7a113
to
1900300
Compare
Co-authored-by: Romaric Pascal <romaric.pascal@digital.cabinet-office.gov.uk>
The string concatenation was a little long to be in the JSX, and required each part to include a leading space Using an Array and join makes things a little easier to follow and less prone to forgetting a space
romaricpascal
force-pushed
the
feature/inputClasses
branch
from
1900300
to
45e6aa2
Compare
There was a problem hiding this comment.
@andreyyudin I ended up managing to push on your branch so I did the edit, as well as two little refactors:
- to rearrange the computation of the input class that was getting a bit lengthy to do with strings
- to simplify the test.
Thanks again for your contribution 😊
|
@romaricpascal Thank you for accepting my contribution and improving it! |
Add input classes to implement a part of #428