[Priority 2] WEBPACK #375
[Priority 2] WEBPACK #375
Conversation
|
@mod @calj @josadcha @VladyslavP @akhlopiachyi @oyershov Tips for reviewing:
|
berezh
requested review from
oyershov,
mod,
akhlopiachyi,
josadcha and
VladyslavP
… into move-to-webpack
|
|
||
| const locale = 'en'; | ||
|
|
||
| export const TestComponentWrapper: React.FC = ({ children }) => { |
There was a problem hiding this comment.
Please, use "App" for this functional, not "TestComponentWrapper"
There was a problem hiding this comment.
This is wrapper for a component which is under the unit test. This is not the application's wrapper. Are you sure to rename it to App?
src/App.tsx
Outdated
| // const lang = useSelector(selectCurrentLanguage); | ||
| // const isMobileDevice = useSelector(selectMobileDeviceState); | ||
| // return ( | ||
| // <IntlProvider | ||
| // {...{ locale: lang }} | ||
| // defaultLocale={lang} | ||
| // messages={getTranslations(lang, isMobileDevice)} | ||
| // key={lang}> | ||
| // <Router history={browserHistory}> | ||
| // <Switch> | ||
| // <Route exact={true} path="/" component={SignInScreen} /> | ||
| // </Switch> | ||
| // </Router> | ||
| // </IntlProvider> | ||
| // ); |
There was a problem hiding this comment.
Please, put here code from "TestComponentWrapper"
There was a problem hiding this comment.
this is test code, I removed it. This App component contains IntlProvider and ReduxProvider wraps it on upper level
| <title>Cryptobase</title> | ||
| </head> | ||
| <body> | ||
| <script src="/config/env.js"></script> |
There was a problem hiding this comment.
Don't you need "%PUBLIC_URL%" prefix? I'm not sure this will work in production, need to make a build and check.
There was a problem hiding this comment.
No, this is webpack's template html page. Relative path is enough.
| @@ -3,7 +3,8 @@ import { | |||
| connect, | |||
| MapStateToProps, | |||
| } from 'react-redux'; | |||
| import { initGeetest } from '../../helpers/geetest'; | |||
| // MUST BE REMOVED: Vadym P. responsibility | |||
There was a problem hiding this comment.
- Why do we need to remove geetest captcha?
- Why it's your responsibility?
There was a problem hiding this comment.
will push fix for geetest to this PR
| @@ -0,0 +1,281 @@ | |||
| // /* eslint-disable */ | |||
There was a problem hiding this comment.
why do we need this to be merged right now to master? If it's some code for the future, make proper PR with it
berezh
force-pushed
the
feature/refactor2
branch
2 times, most recently
from
2205fee
to
10cd24a
Compare
berezh
changed the base branch from
feature/refactor2
to
feature/refactor-vp
No description provided.