-
Notifications
You must be signed in to change notification settings - Fork 59
Update issue reporting template #538
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
Conversation
binh-dam-ibigroup
left a comment
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.
Conditional approval upon merging #542.
…emplate-qbd Update issue reporting template qbd
daniel-heppner-ibigroup
left a comment
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.
Some code structure thoughts... but still approving since my opinion might not be the best 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.
This feels like it would work well as a functional component.
| const { buttonTypes, reactRouterConfig, reportConfig } = this.props | ||
|
|
||
| const buttonComponents = [] | ||
| buttonTypes.forEach((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.
How about a map to map buttonTypes to buttonComponents?
|
You're definitely right on both fronts and I think we should do both of these fixes when we move this file to Typescript, hopefully soon! |
The previous issue reporting email template left a lot to be desired in terms of user experience. This PR attempts to make the template more user friendly.
French translations are now obsolete and need to be updated @binh-dam-ibigroup