-
Notifications
You must be signed in to change notification settings - Fork 19
Refactor colors to use theme tokens #300
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,4 @@ | ||
| import { StyleSheet, Text, TouchableOpacity, View } from 'react-native'; | ||
| import { Colors } from '../../utils/colors'; | ||
| import { InterRegular, InterSemiBold } from '../../utils/webFonts'; | ||
| import { Text, Pressable, View } from 'native-base'; | ||
| import { DonorCollective } from '../../models/models'; | ||
| import useCrossNavigate from '../../routes/useCrossNavigate'; | ||
| import { formatAddress } from '../../lib/formatAddress'; | ||
|
|
@@ -23,27 +21,31 @@ export const DonorsListItem = ({ donor, rank, userFullName }: DonorsListItemProp | |
| const userIdentifier = userFullName ?? ensName ?? formatAddress(donor.donor); | ||
|
|
||
| const circleBackgroundColor = | ||
| rank === 1 ? Colors.yellow[100] : rank === 2 ? Colors.gray[700] : rank === 3 ? Colors.orange[400] : 'none'; | ||
| rank === 1 ? 'goodYellow.100' : rank === 2 ? 'goodGrey.700' : rank === 3 ? 'goodOrange.350' : 'none'; | ||
| const circleTextColor = | ||
| rank === 1 ? Colors.yellow[200] : rank === 2 ? Colors.blue[200] : rank === 3 ? Colors.brown[100] : Colors.black; | ||
| rank === 1 ? 'goodYellow.200' : rank === 2 ? 'goodBlue.200' : rank === 3 ? 'goodBrown.100' : 'black'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unmemoized Color Calculations
Tell me moreWhat is the issue?Multiple ternary operations for color selection are recalculated on every render, even though they only depend on the rank prop. Why this mattersThese color calculations could be memoized since they only depend on the rank prop, avoiding unnecessary recalculations during re-renders. Suggested change ∙ Feature PreviewUse useMemo to cache the color calculations: const colors = useMemo(() => ({
background: rank === 1 ? 'goodYellow.100' : rank === 2 ? 'goodGrey.700' : rank === 3 ? 'goodOrange.350' : 'none',
text: rank === 1 ? 'goodYellow.200' : rank === 2 ? 'goodBlue.200' : rank === 3 ? 'goodBrown.100' : 'black'
}), [rank]);Provide feedback to improve future suggestions💬 Looking for more details? Reply to this comment to chat with Korbit. |
||
|
|
||
| return ( | ||
| <TouchableOpacity style={styles.rowBetween} onPress={() => navigate(`/profile/${donor.donor}`)}> | ||
| <View style={styles.rowTogether}> | ||
| <View style={[styles.circle, { backgroundColor: circleBackgroundColor }]}> | ||
| <Text style={[styles.circleText, { color: circleTextColor }]}>{rank}</Text> | ||
| <Pressable {...styles.rowBetween} onPress={() => navigate(`/profile/${donor.donor}`)}> | ||
| <View {...styles.rowTogether}> | ||
| <View {...styles.circle} style={{ backgroundColor: circleBackgroundColor }}> | ||
| <Text {...styles.circleText} style={{ color: circleTextColor }}> | ||
| {rank} | ||
| </Text> | ||
| </View> | ||
| <Text style={[styles.title, { color: circleTextColor }]}>{userIdentifier}</Text> | ||
| <Text {...styles.title} style={{ color: circleTextColor }}> | ||
| {userIdentifier} | ||
| </Text> | ||
| </View> | ||
| <Text style={styles.totalDonated}> | ||
| <Text style={styles.currency}>G$ </Text> | ||
| <Text {...styles.totalDonated}> | ||
| <Text {...styles.currency}>G$ </Text> | ||
| <GoodDollarAmount amount={formattedDonations || '0'} /> | ||
| </Text> | ||
| </TouchableOpacity> | ||
| </Pressable> | ||
| ); | ||
| }; | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| const styles = { | ||
| circle: { | ||
| paddingVertical: 2, | ||
| paddingHorizontal: 10, | ||
|
|
@@ -52,17 +54,17 @@ const styles = StyleSheet.create({ | |
| circleText: { | ||
| lineHeight: 24, | ||
| fontSize: 16, | ||
| ...InterRegular, | ||
| fontWeight: 500, | ||
| }, | ||
| rowNumber: { | ||
| fontSize: 16, | ||
| lineHeight: 24, | ||
| color: Colors.black, | ||
| ...InterRegular, | ||
| color: 'black', | ||
| fontWeight: 500, | ||
| }, | ||
| rowBetween: { | ||
| width: '100%', | ||
| backgroundColor: Colors.white, | ||
| backgroundColor: 'white', | ||
| flex: 1, | ||
| flexDirection: 'row', | ||
| marginVertical: 8, | ||
|
|
@@ -77,27 +79,27 @@ const styles = StyleSheet.create({ | |
| row: { | ||
| minHeight: 48, | ||
| width: '100%', | ||
| backgroundColor: Colors.white, | ||
| backgroundColor: 'white', | ||
| flex: 1, | ||
| flexDirection: 'row', | ||
| marginVertical: 8, | ||
| alignItems: 'center', | ||
| }, | ||
| title: { | ||
| fontSize: 16, | ||
| ...InterSemiBold, | ||
| fontWeight: 700, | ||
| width: '100%', | ||
| color: Colors.black, | ||
| color: 'black', | ||
| marginLeft: 8, | ||
| }, | ||
| totalDonated: { | ||
| fontSize: 14, | ||
| ...InterRegular, | ||
| fontWeight: 500, | ||
| textAlign: 'right', | ||
| width: '100%', | ||
| color: Colors.gray[100], | ||
| color: 'goodGrey.400', | ||
| }, | ||
| currency: { | ||
| ...InterSemiBold, | ||
| fontWeight: 700, | ||
| }, | ||
| }); | ||
| } as const; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid maxHeight value
Tell me more
What is the issue?
The cardDescription style uses 'auto' as a string value for maxHeight, which is not a valid value for this property
Why this matters
Invalid maxHeight value will be ignored by the layout engine, potentially causing text overflow issues
Suggested change ∙ Feature Preview
Either remove the maxHeight property if no limit is needed, or set a specific numeric value:
Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.