Skip to content
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.

Sign up for GitHub

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

fix: fix reply border style #2197

Merged
merged 6 commits into from
Sep 4, 2023
Merged

fix: fix reply border style #2197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
12b0cfb
fix: fix reply border
aharwood9 Jul 24, 2023
8c9ca8d
chore: revert move of MessageRepliesAvatars
aharwood9 Jul 24, 2023
fc18426
chore: revert styling
aharwood9 Jul 24, 2023
a025150
chore: update snapshot
aharwood9 Jul 24, 2023
2019065
chore: remove margin
aharwood9 Sep 3, 2023
5b03fe7
chore: update snapshot
aharwood9 Sep 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 6 additions & 17 deletions package/src/components/Message/MessageSimple/MessageReplies.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { ColorValue, Platform, StyleSheet, Text, TouchableOpacity, View } from 'react-native';
import { ColorValue, StyleSheet, Text, TouchableOpacity, View } from 'react-native';

import {
MessageContextValue,
Expand Down Expand Up @@ -28,15 +28,9 @@ const styles = StyleSheet.create({
},
leftMessageRepliesCurve: {
borderBottomLeftRadius: 16,
borderRightColor: 'transparent',
...Platform.select({
android: {
borderRightWidth: 0,
},
}),
borderRightWidth: 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If its 0, if there a need to have this property in the styles anymore?

},
messageRepliesCurve: {
borderTopColor: 'transparent',
borderTopWidth: 0,
borderWidth: 1,
height: 16,
Expand All @@ -46,16 +40,11 @@ const styles = StyleSheet.create({
fontSize: 12,
fontWeight: '700',
paddingBottom: 5,
paddingLeft: 8,
paddingHorizontal: 8,
},
rightMessageRepliesCurve: {
borderBottomRightRadius: 16,
borderLeftColor: 'transparent',
...Platform.select({
android: {
borderLeftWidth: 0,
},
}),
borderLeftWidth: 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If its 0, if there a need to have this property in the styles anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the current structure first passes styles.messageRepliesCurve with a borderWidth: 1. Then, we remove the opposite side border within each rightMessageRepliesCurve or leftMessageRepliesCurve. I've attached an image if we just remove that line.

Screenshot 2023-09-03 at 14 38 34

However, if this is not ideal, we could remove the borderWidth: 1 from the styles.messageRepliesCurve and replace it with borderWithBottom: 1 and then have a left or right borderWidth with each replies curve. This should give the same impact.

},
});

Expand Down Expand Up @@ -112,7 +101,7 @@ const MessageRepliesWithContext = <
return (
<View style={styles.curveContainer}>
{alignment === 'left' && (
<View testID='message-replies-left'>
<View style={styles.curveContainer} testID='message-replies-left'>
{!noBorder && (
<View
style={[
Expand Down Expand Up @@ -166,7 +155,7 @@ const MessageRepliesWithContext = <
</Text>
</TouchableOpacity>
{alignment === 'right' && (
<View testID='message-replies-right'>
<View style={styles.curveContainer} testID='message-replies-right'>
<MessageRepliesAvatars alignment={alignment} message={message} />
{!noBorder && (
<View
Expand Down
2 changes: 1 addition & 1 deletion package/src/components/Message/MessageSimple/MessageRepliesAvatars.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const MessageRepliesAvatarsWithContext = <
<View
style={[
styles.avatarContainer,
alignment === 'right' ? { marginLeft: 8, ...rightAvatarsContainer } : leftAvatarsContainer,
alignment === 'right' ? rightAvatarsContainer : leftAvatarsContainer,
]}
>
{avatars.map((user, i) => (
Expand Down
10 changes: 7 additions & 3 deletions .../src/components/Message/MessageSimple/__tests__/__snapshots__/MessageReplies.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ exports[`MessageReplies should render message replies 1`] = `
}
>
<View
style={
Object {
"flexDirection": "row",
}
}
testID="message-replies-left"
>
<View
Expand All @@ -18,15 +23,14 @@ exports[`MessageReplies should render message replies 1`] = `
"borderColor": undefined,
},
Object {
"borderTopColor": "transparent",
"borderTopWidth": 0,
"borderWidth": 1,
"height": 16,
"width": 16,
},
Object {
"borderBottomLeftRadius": 16,
"borderRightColor": "transparent",
"borderRightWidth": 0,
},
Object {},
]
Expand Down Expand Up @@ -73,7 +77,7 @@ exports[`MessageReplies should render message replies 1`] = `
"fontSize": 12,
"fontWeight": "700",
"paddingBottom": 5,
"paddingLeft": 8,
"paddingHorizontal": 8,
},
Object {
"color": "#005FFF",
Expand Down