Begin porting the web POC to react native#10
Conversation
AndrewGable
left a comment
There was a problem hiding this comment.
Web works great and the code is very clean! I left some comments from my testing on iOS. I am totally happy leaving the harder ones as TODOs in the code if we mark them as such and looking at them later.
src/style/StyleSheet.js
Outdated
| padding: 8, | ||
| }, | ||
| mainContentWrapper: { | ||
| height: 'calc(100vh - 73px)', |
There was a problem hiding this comment.
Mobile does not like this line, it chokes on it. Is there anyway to refactor it to not use calc?
There was a problem hiding this comment.
Ah phooey. I can keep trying, yeah.
src/Expensify.js
Outdated
| <Route path="/" component={HomePage} /> | ||
| </Switch> | ||
| </Router> | ||
| <Beforeunload onBeforeunload={ActiveClientManager.removeClient}> |
There was a problem hiding this comment.
Mobile doesn't play well with this, it seems to use window quite a bit.
There was a problem hiding this comment.
Dang, this was supposed to be compatible :(
There was a problem hiding this comment.
I am not an expert on this event, but could we use AppState instead? It is supported by RN and RN4W out of the box: https://github.com/necolas/react-native-web#modules
There was a problem hiding this comment.
I just tested this and yes it does change to inactive when the tab is changed, then back to active when it comes back. This sounds like possibly we need to handle it using a native/web solution (like Router).
| const myPersonalDetails = allPersonaDetails[currentLogin]; | ||
| return Store.multiSet({ | ||
| [STOREKEYS.PERSONAL_DETAILS]: allPersonaDetails, | ||
| [STOREKEYS.MY_PERSONAL_DETAILS]: myPersonalDetails, |
There was a problem hiding this comment.
Not sure if you see this on web but iOS complains whenever something is set that is undefined (as this is on boot), not sure if or when we want to set a default value, but it could keep the logs from being too noisy if we added one.
There was a problem hiding this comment.
I don't think I've seen those errors, no. What is undefined when that happens. Is it myPersonalDetails or allPersonaDetails (which I just realized has a typo)
There was a problem hiding this comment.
It was myPersonalDetails that was undefined
Co-authored-by: Andrew Gable <andrewgable@gmail.com>
Co-authored-by: Andrew Gable <andrewgable@gmail.com>
…eactNativeChat into tgolen-port-webpoc
src/page/HomePage/SidebarLink.js
Outdated
| <Text style={textActiveStyle}>{this.props.reportName}</Text> | ||
| {this.state.isUnread && ( | ||
| <Text style={textActiveStyle}>- Unread</Text> | ||
| )} |
There was a problem hiding this comment.
Not exactly clear why, but this needs to be wrapped in a view for iOS to work.
| <Text style={textActiveStyle}>{this.props.reportName}</Text> | |
| {this.state.isUnread && ( | |
| <Text style={textActiveStyle}>- Unread</Text> | |
| )} | |
| <View> | |
| <Text style={textActiveStyle}>{this.props.reportName}</Text> | |
| {this.state.isUnread && ( | |
| <Text style={textActiveStyle}>- Unread</Text> | |
| )} | |
| </View> |
src/style/StyleSheet.js
Outdated
| }, | ||
| sidebarLinkActive: { | ||
| backgroundColor: '#007bff', | ||
| borderRadius: '.25rem', |
There was a problem hiding this comment.
iOS chokes on rem values too 😢
|
It's worth a shot. I'm curious if that will fire on web when the tab
gains/loses focus... which might not really be what we want.
…On Sun, Aug 9, 2020 at 5:13 PM Andrew Gable ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/Expensify.js
<#10 (comment)>
:
> }
render() {
return (
- <Router>
- {/* If there is ever a property for redirecting, we do the redirect here */}
- {this.state.redirectTo && <Redirect to={this.state.redirectTo} />}
-
- <Switch>
- <Route path="/signin" component={SignInPage} />
- <Route path="/" component={HomePage} />
- </Switch>
- </Router>
+ <Beforeunload onBeforeunload={ActiveClientManager.removeClient}>
I am not an expert on this event, but could we use AppState instead? It
is supported by RN and RN4W out of the box:
https://github.com/necolas/react-native-web#modules
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#10 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJMAB6W752QPOZVGDS5W4DR74UR7ANCNFSM4PY7Y6VQ>
.
|
|
🚀 [Deployed](https://github.com/Expensify/Expensify.cash
|
|
🚀 Deployed to staging by https://github.com/AndrewGable in version: 1.3.77-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.3.77-7 🚀
|
* fix: ubo list hover, incorp state hover * feat: improved return type for getFieldRequiredErrors
…-settings Improve workspace settings on Main Pane
Deploying helpdot with
|
| Latest commit: |
4f305ce
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://36897b35.helpdot.pages.dev |
…tion-report-view Revert "Add One Transaction Report View"
Lots of stuff in this!