-
-
Notifications
You must be signed in to change notification settings - Fork 245
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
Added custom ListField icons in bootstrap4. #114
Conversation
add ability to include custom add / remove icons for ListFields
@@ -8,6 +8,7 @@ const ListAdd = ({ | |||
disabled, | |||
parent, | |||
value, | |||
addIcon = (<i className="octicon octicon-plus" />), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, use defaultProps
instead.
@@ -8,6 +8,7 @@ const ListDel = ({ | |||
disabled, | |||
name, | |||
parent, | |||
removeIcon = (<i className="octicon octicon-dash" />), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
@@ -16,6 +16,8 @@ const List = ({ | |||
label, | |||
name, | |||
value, | |||
addIcon, | |||
removeIcon, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's not in the ESLint, but please, keep them sorted.
Great! |
I don't use BS3, but it should be relatively simple, I will create a separate PR for BS3. |
@@ -24,4 +24,8 @@ const ListAdd = ({ | |||
); | |||
}; | |||
|
|||
ListAdd.defaultProps = { | |||
addIcon: (<i className="octicon octicon-plus" />), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for being so picky (just say no, if you want), but could you remove the parentheses and trailing comma?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure thing, we each have our own style, I'll try to match yours. FYI my thought process behind the comma: I prefer commas after the final line so when you add a property at the end of an object, the commit is only 1 line change, not 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why I had the parentheses, I'll get rid of that too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I would go with less-diff-is-better ideology, I wouldn't align assignements, imports, properties and so on... Currently it's not in our ESLint, so I'm forcing my code style here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want the defaultProps all one line (just 1 property), or multi ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multi.
@@ -28,4 +28,8 @@ const ListDel = ({ | |||
); | |||
}; | |||
|
|||
ListDel.defaultProps = { | |||
removeIcon: (<i className="octicon octicon-dash" />), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
code style change for defaultProps
I'll release |
Add ability to include custom add / remove icons for ListFields.
This can be implemented in either AutoField or by using ListField.
Example AutoField:
Example ListField: