-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
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.
👍
currentId: PropTypes.number, | ||
maxId: PropTypes.number, | ||
className: PropTypes.string, | ||
right: PropTypes.bool, |
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.
https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/sort-prop-types.md
Should we ? (PS, r
is after o
😆)
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.
Funnily enough, I played with that some weeks ago - it does some nice stuff, e.g. required first, then normal, then handlers (on
Something) - obviously all ordered. (Or you can just order without caring about the type)
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.
So shouldn't we use it ?
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.
We should, the only reason I have not pulled it in as part of one of my eslint updates (yet) is that we are basically wrong in 70% of the cases. So lot of small little niggly updates, just didn't have the time to fiddle.
Medium-term, I would love to.
(Split from https://github.com/ethcore/parity/pull/4539)
~/ui/Tooltips/Tooltip
FormattedMessage
where Tooltips are constructed