Fix CalendarList rendering on react-native-web #1313
Conversation
src/agenda/index.js
Outdated
| @@ -35,7 +31,7 @@ export default class AgendaView extends Component { | |||
| /** Specify theme properties to override specific styles for calendar parts. Default = {} */ | |||
| theme: PropTypes.object, | |||
| /** agenda container style */ | |||
| style: viewPropTypes.style, | |||
| style: PropTypes.object, | |||
There was a problem hiding this comment.
'style' prop type should be PropTypes.oneOfType([PropTypes.object, PropTypes.number, PropTypes.array]) as you can pass style inline as an object or array and you can pass it from a StyleSheet where it's just a number.
| typeof document !== 'undefined' | ||
| ? PropTypes.shape({style: PropTypes.object}) | ||
| : ViewPropTypes || View.propTypes; | ||
| const {Text, View, Dimensions, Animated, Platform} = ReactNative; |
There was a problem hiding this comment.
Why is 'import * as ReactNative from 'react-native';' still necessary? Can't we extract the components from 'react-native' without importing it all?
There was a problem hiding this comment.
I was trying to be as non-invasive as possible - I'm new to JS / React. Would you like me to remove the 'import * as ReactNative' and fix anything that depends on it?
There was a problem hiding this comment.
I do want to remove the * import but I don't want to break anyone. Can you tell me how do you test this in web on old RN versions?
There was a problem hiding this comment.
Err.. I haven't tested on older RN versions. Good idea. I'll make sure that works then post back here.
| typeof document !== 'undefined' | ||
| ? PropTypes.shape({style: PropTypes.object}) | ||
| : ViewPropTypes || View.propTypes; | ||
| const {View} = ReactNative; |
There was a problem hiding this comment.
Also here. Why keep importing * from 'react-native'?
| @@ -119,6 +115,8 @@ export default class AgendaView extends Component { | |||
| }; | |||
|
|
|||
| this.currentMonth = this.state.selectedDay.clone(); | |||
| this.onHover = this.onHover.bind(this); | |||
There was a problem hiding this comment.
What these changes have to do with fixing the rendering? I think you're mixing two tasks here...
There was a problem hiding this comment.
This adds mouse support so that the interactions can work on web. I stole this from here and credited the author in the main PR comment.
| @@ -294,7 +291,7 @@ class CalendarList extends Component { | |||
|
|
|||
| render() { | |||
| return ( | |||
| <View> | |||
| <View style={{flex: 1}}> | |||
There was a problem hiding this comment.
This is what actually fixes the rendering - FlatList scrolling only works on web if the parent component has a flex style. Source: necolas/react-native-web#1436 (comment)
There was a problem hiding this comment.
If that's a style to fix a web rendering you should add it to a StyleSheet with Platform division. You can't change for all Platforms to fix one...
There was a problem hiding this comment.
@tedsta Per @Inbal-Tish's comment, this line should look something like:
import { StyleSheet, Platform } from 'react-native'
...
render() {
return <View style={Platform.select({ web: styles.webContainer, default: undefined })}>
}
const styles = StyleSheet.create({
webContainer: {
flex: 1
}
})Or, like this:
<View style={styles.container}>
const styles = StyleSheet.create({
container: Platform.select({ web: { flex: 1 }, default: {} })
})
| @@ -139,6 +146,16 @@ export default class AgendaView extends Component { | |||
| this.state.scrollY.removeAllListeners(); | |||
| } | |||
|
|
|||
| onHover() { | |||
There was a problem hiding this comment.
These are very specific implementations. If you're adding a feature, first, you should make a separate PR for that and not add it to another issue. Second, you should write an API that lets the user adjust these implementations if required.
There was a problem hiding this comment.
Sure I'll split the mouse interactions into a second PR. Unless @bkniffler wants it since it is their code :) LMK
For API/adjustable-ness - none of the other interactions have this AFAICT? Am I missing something?
There was a problem hiding this comment.
I don't think these hover interactions are crucial for web support. In my opinion, they should be moved to a separate PR to help get this one merged sooner.
The right way to do this would probably be to transfer over to React Native's new Pressable (available in 0.63), and style it like so:
<Pressable
// react-native-web 0.14 added support for hovered here 🔥
style={({ hovered, pressed }) => {
opacity: pressed || hovered ? 0.7 : 1
}}
/>I'm not sure how focused on backwards-compatibility this library is, though. So this might be a later priority.
| //initialScrollIndex={1} | ||
| onScrollToIndexFailed={() => {}} |
There was a problem hiding this comment.
Please remove the comment.
Is onScrollToIndexFailed={() => {}} necessary? It does nothing.
|
Keen to install this after PR is merged :-) |
|
Thank you guys for your hard work on this, looking forward to installing it when it's merged! |
|
Hi guys, I was wondering if there was some way to patch this from our side while this issue is being resolved? Is there a way to access the necessary view to add |
|
Sorry guys, I'm in a new job now and don't work with React anymore 😧 Hopefully one of you guys can fork this and push it over the finish line. I'll leave this open but feel free to close. |
|
Following the last comment I opened this PR: #1405 |
This fixes CalendarList / Agenda rendering when running on web via Expo.
Thanks to @bkniffler with his fix for touch support on web here: https://github.com/bkniffler/react-native-calendars/commit/7a0f986bbef1f84b0fd6901af6974c8bcd7436c2
The main issue was that
FlatListscrolling only works on web if the parent component has aflexstyle. Source: necolas/react-native-web#1436 (comment)Also incorporated fixes for some
PropTypeserrors on web, thanks @nandorojo: #1033 (comment)Fixes #924