-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Convert modals to use JSX #7566
Conversation
Interesting lint failure here 🤔 looks like we're checking for single-quoted strings, specifically |
b32322a
to
b39476c
Compare
development/verify-locale-strings.js
Outdated
@@ -161,7 +161,7 @@ async function verifyEnglishLocale (fix = false) { | |||
const englishLocale = await getLocale('en') | |||
const javascriptFiles = await findJavascriptFiles(path.resolve(__dirname, '..', 'ui')) | |||
|
|||
const regex = /'(\w+)'/g | |||
const regex = /['"](\w+)["']/g |
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 will catch some false positives, like "example's"
would match example
despite the beginning quote being different from the end quote.
Something like this would work better:
const regex = /['"](\w+)["']/g | |
const regex = /'(\w+)'|"(\w+)"/g |
However that'd create two capture groups. So the match[1]
would need to be replaced with something like
match[1] || match[2]
h(DepositEtherModal, {}, []), | ||
], | ||
contents: [( | ||
<DepositEtherModal key={0} /> |
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.
Nit: All of these single-entry arrays could be assigned the component directly instead. This contents
is just passed as the children
prop to the FadeModal
- it's never assumed to be an array.
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.
Good point, I'll update these
b9e0702
to
5018330
Compare
5018330
to
117e3ba
Compare
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.
LGTM
Refs #6291
This PR replaces hyperscript in the modals file with JSX.