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
[Dialog] Fix typo and finish incomplete comment #9379
Conversation
I finished what I thought the original commenter wanted to say.
src/Dialog/DialogTitle.js
Outdated
@@ -42,7 +42,7 @@ export type Props = { | |||
className?: string, | |||
/** | |||
* If `true`, the children won't be wrapped by a typography component. | |||
* For instance, that can be useful to can render an h4 instead of a | |||
* For instance, that can be useful to render a h4 instead of the default h2. |
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.
Looks good, except the original "an" was correct, at least in American English.
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.
And in English English. 😄
I would also change 'that' to 'this'.
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.
Also, please cloud you update the base commit title to match the PR title (as it is now) - it keeps make preparing the CHANGELOG much easier! Thanks.
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.
Also, please cloud you update the base commit title to match the PR title (as it is now) - it keeps make preparing the CHANGELOG much easier! Thanks.
No neeed. We can squash & rebase. We can put whatever we prefer as admin.
Looking at this a bit closer, I wonder what the rational was behind |
@mbrookes People might want to use react-intl to translate the title. However, I'm wondering about at |
I like that idea better. |
@willgriffiths Thank you, it's a first great pull-request here :). |
I finished what I thought the original commenter wanted to say. Thanks for you guys hard work. This project is awesome!