Changeset View
Standalone View
native/chat/inline-sidebar.react.js
| // @flow | // @flow | ||||
| import * as React from 'react'; | import * as React from 'react'; | ||||
| import { Text, View } from 'react-native'; | import { Text, View } from 'react-native'; | ||||
| import useInlineEngagementText from 'lib/hooks/inline-engagement-text.react'; | import useInlineEngagementText from 'lib/hooks/inline-engagement-text.react'; | ||||
| import type { ThreadInfo } from 'lib/types/thread-types'; | import type { ThreadInfo } from 'lib/types/thread-types'; | ||||
| import Button from '../components/button.react'; | import Button from '../components/button.react'; | ||||
| import { useStyles } from '../themes/colors'; | import { useStyles } from '../themes/colors'; | ||||
| import { inlineSidebarHeight } from './chat-constants'; | import { inlineSidebarHeight } from './chat-constants'; | ||||
| import { useNavigateToThread } from './message-list-types'; | import { useNavigateToThread } from './message-list-types'; | ||||
| type Props = { | type Props = { | ||||
| +threadInfo: ThreadInfo, | +threadInfo: ThreadInfo, | ||||
| +position: 'left' | 'right', | |||||
| }; | }; | ||||
| function InlineSidebar(props: Props): React.Node { | function InlineSidebar(props: Props): React.Node { | ||||
| const { threadInfo } = props; | const { threadInfo, position } = props; | ||||
| const { repliesText } = useInlineEngagementText(threadInfo); | const { repliesText } = useInlineEngagementText(threadInfo); | ||||
| const navigateToThread = useNavigateToThread(); | const navigateToThread = useNavigateToThread(); | ||||
| const onPress = React.useCallback(() => { | const onPress = React.useCallback(() => { | ||||
| navigateToThread({ threadInfo }); | navigateToThread({ threadInfo }); | ||||
| }, [navigateToThread, threadInfo]); | }, [navigateToThread, threadInfo]); | ||||
| const styles = useStyles(unboundStyles); | const styles = useStyles(unboundStyles); | ||||
| const unreadStyle = threadInfo.currentUser.unread ? styles.unread : null; | const unreadStyle = threadInfo.currentUser.unread ? styles.unread : null; | ||||
| const contentStyles = [styles.content, styles[position]]; | |||||
| return ( | return ( | ||||
| <View style={styles.content}> | <View style={contentStyles}> | ||||
| <Button style={styles.sidebar} onPress={onPress}> | <Button style={styles.sidebar} onPress={onPress}> | ||||
| <Text style={[styles.name, unreadStyle]}>{repliesText}</Text> | <Text style={[styles.name, unreadStyle]}>{repliesText}</Text> | ||||
| </Button> | </Button> | ||||
| </View> | </View> | ||||
| ); | ); | ||||
| } | } | ||||
| const unboundStyles = { | const unboundStyles = { | ||||
| content: { | content: { | ||||
| flexDirection: 'row', | flexDirection: 'row', | ||||
| flex: 1, | |||||
| height: inlineSidebarHeight, | height: inlineSidebarHeight, | ||||
| backgroundColor: 'inlineEngagementBackground', | |||||
| borderRadius: 28, | |||||
| padding: 8, | |||||
| position: 'absolute', | |||||
| bottom: -29, | |||||
| }, | }, | ||||
| unread: { | unread: { | ||||
| color: 'listForegroundLabel', | color: 'listForegroundLabel', | ||||
| fontWeight: 'bold', | fontWeight: 'bold', | ||||
| }, | }, | ||||
| sidebar: { | sidebar: { | ||||
| flexDirection: 'row', | flexDirection: 'row', | ||||
| display: 'flex', | display: 'flex', | ||||
| alignItems: 'center', | alignItems: 'center', | ||||
| }, | }, | ||||
| icon: { | icon: { | ||||
| color: 'listForegroundTertiaryLabel', | color: 'listForegroundTertiaryLabel', | ||||
| }, | }, | ||||
| name: { | name: { | ||||
| color: 'listForegroundLabel', | |||||
| paddingTop: 1, | paddingTop: 1, | ||||
| color: 'listForegroundTertiaryLabel', | fontSize: 14, | ||||
| fontSize: 16, | |||||
| paddingLeft: 4, | paddingLeft: 4, | ||||
| paddingRight: 2, | paddingRight: 2, | ||||
| }, | }, | ||||
| left: { | |||||
| left: 0, | |||||
| }, | |||||
| right: { | |||||
| right: 20, | |||||
ashoat: Why do we need `right: 20` but don't need `left: 0`? | |||||
benschacAuthorUnsubmitted I'm pretty sure it has to do with how absolute positioning works. We could remove left: 0, and it would still work. I'm not 100% how it works in react-native land. But on web absolute positioning defaults to top: 0, left: 0 benschac: I'm pretty sure it has to do with how absolute positioning works. We could remove `left: 0`… | |||||
benschacAuthorUnsubmitted which also seems to be the case with react-native. benschac: which also seems to be the case with react-native. | |||||
ashoatUnsubmitted But why isn't it left: 20 to match up with right: 20? I'm trying to understand why these are different. Does the design call for this difference? Is it different because we're accounting for the deliveryIcon? If we're using 20 in multiple places, it feels like we should avoid hardcoding it and instead to put it as a constant somewhere. Even better would be if we structured the view hierarchy so that the delivery icon is outside of the InlineEngagement, but that might be really hard... Definitely still possible that no changes are needed here, but requested changes again to get additional clarification ashoat: But why isn't it `left: 20` to match up with `right: 20`? I'm trying to understand why these… | |||||
benschacAuthorUnsubmitted
Without right:  Absolute position is default top: 0, left: 0 in reality. Technically it's all auto, but top: 0, left: 0 is how it behaves, shown in the codepen linked above: https://codepen.io/benschac/pen/OJQxoYa. If we define a right: <number> the element will position right <number> pixels of the closest relative positioned parent element. left: 0 is because the element that's the relative parent is still pushed off the edge of the screen by some margin. So, there's no additional need to move it farther. If we add left: 20  the other thing to account for is left doesn't have the send check icon that right does have. benschac: > But why isn't it left: 20 to match up with right: 20? I'm trying to understand why these are… | |||||
ashoatUnsubmitted This is the only relevant part of your response:
I guessed that this was the case, and so preemptively asked you some follow-up questions in my previous comment. Can you please respond to those follow-ups? For convenience, I will quote them here:
ashoat: This is the only relevant part of your response:
> the other thing to account for is left… | |||||
| }, | |||||
| }; | }; | ||||
| export default InlineSidebar; | export default InlineSidebar; | ||||
Why do we need right: 20 but don't need left: 0?