-
Notifications
You must be signed in to change notification settings - Fork 4k
Migrate Button to ButtonComposed (batch 8) #97089
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
base: main
Are you sure you want to change the base?
Changes from all commits
70eb774
ee1f4c7
a6b7e0a
54361cf
cca8558
ae3274b
5dbdb3f
ced02d5
bb69012
697bb2f
df56236
e196f65
7a710c1
dd2a112
d5410bd
3457ef7
87add1f
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,5 +1,5 @@ | ||||||
| import type {Attachment} from '@components/Attachments/types'; | ||||||
| import Button from '@components/Button'; | ||||||
| import Button from '@components/ButtonComposed'; | ||||||
| import Tooltip from '@components/Tooltip'; | ||||||
|
|
||||||
| import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; | ||||||
|
|
@@ -51,31 +51,39 @@ function CarouselButtons({page, attachments, shouldShowArrows, onBack, onForward | |||||
| <Tooltip text={translate('common.previous')}> | ||||||
| <View style={[styles.attachmentArrow, shouldUseNarrowLayout ? styles.l2 : styles.l8]}> | ||||||
| <Button | ||||||
| small | ||||||
| innerStyles={[styles.arrowIcon]} | ||||||
| icon={icons.BackArrow} | ||||||
| iconFill={theme.text} | ||||||
| size={CONST.BUTTON_SIZE.SMALL} | ||||||
| innerStyles={styles.arrowIcon} | ||||||
| onPress={onBack} | ||||||
| onPressIn={cancelAutoHideArrow} | ||||||
| onPressOut={autoHideArrow} | ||||||
| sentryLabel={CONST.SENTRY_LABEL.ATTACHMENT_CAROUSEL.PREVIOUS_BUTTON} | ||||||
| /> | ||||||
| > | ||||||
| <Button.Icon | ||||||
| src={icons.BackArrow} | ||||||
| fill={theme.text} | ||||||
| hoverFill={theme.text} | ||||||
| /> | ||||||
| </Button> | ||||||
| </View> | ||||||
| </Tooltip> | ||||||
| )} | ||||||
| {!isForwardDisabled && ( | ||||||
| <Tooltip text={translate('common.next')}> | ||||||
| <View style={[styles.attachmentArrow, shouldUseNarrowLayout ? styles.r2 : styles.r8]}> | ||||||
| <Button | ||||||
| small | ||||||
| size={CONST.BUTTON_SIZE.SMALL} | ||||||
| innerStyles={[styles.arrowIcon]} | ||||||
|
Contributor
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. NIT:
Suggested change
|
||||||
| icon={icons.ArrowRight} | ||||||
| iconFill={theme.text} | ||||||
| onPress={onForward} | ||||||
| onPressIn={cancelAutoHideArrow} | ||||||
| onPressOut={autoHideArrow} | ||||||
| sentryLabel={CONST.SENTRY_LABEL.ATTACHMENT_CAROUSEL.NEXT_BUTTON} | ||||||
| /> | ||||||
| > | ||||||
| <Button.Icon | ||||||
| src={icons.ArrowRight} | ||||||
| fill={theme.text} | ||||||
| hoverFill={theme.text} | ||||||
| /> | ||||||
| </Button> | ||||||
| </View> | ||||||
| </Tooltip> | ||||||
| )} | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import AttachmentView from '@components/Attachments/AttachmentView'; | ||
| import type {Attachment} from '@components/Attachments/types'; | ||
| import Button from '@components/Button'; | ||
| import Button from '@components/ButtonComposed'; | ||
| import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback'; | ||
| import SafeAreaConsumer from '@components/SafeAreaConsumer'; | ||
| import Text from '@components/Text'; | ||
|
|
@@ -48,8 +48,10 @@ function CarouselItem({item, onPress, isFocused, isModalHovered, reportID}: Caro | |
|
|
||
| const renderButton = (style: StyleProp<ViewStyle>) => ( | ||
| <Button | ||
|
Contributor
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. On
Member
Author
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. Thanks! I tested all three buttons in German and with an artificially long label and saw no overflow, so I decided I'll leave it as it is |
||
| small | ||
| size={CONST.BUTTON_SIZE.SMALL} | ||
| style={style} | ||
| // Restores the 12px horizontal padding from the legacy implementation. | ||
| innerStyles={styles.ph3} | ||
|
Contributor
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. Could you add a short comment, for example: // Raw children skip ButtonText and the ph1 it contributes, so the legacy 12px horizontal padding is restored here.
innerStyles={styles.ph3} |
||
| onPress={() => setIsHidden(!isHidden)} | ||
| testID="moderationButton" | ||
| sentryLabel={CONST.SENTRY_LABEL.ATTACHMENT_CAROUSEL.MODERATION_BUTTON} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import Button from '@components/Button'; | ||
| import Button from '@components/ButtonComposed'; | ||
| import OfflineWithFeedback from '@components/OfflineWithFeedback'; | ||
| import Text from '@components/Text'; | ||
|
|
||
|
|
@@ -10,7 +10,7 @@ import {getOriginalMessage} from '@libs/ReportActionsUtils'; | |
|
|
||
| import {removeEvent} from '@userActions/Chronos'; | ||
|
|
||
| import type CONST from '@src/CONST'; | ||
| import CONST from '@src/CONST'; | ||
| import type ReportAction from '@src/types/onyx/ReportAction'; | ||
|
|
||
| import React from 'react'; | ||
|
|
@@ -61,8 +61,10 @@ function ChronosOOOListActions({reportID, action}: ChronosOOOListActionsProps) { | |
| )} | ||
| </Text> | ||
| <Button | ||
| small | ||
| size={CONST.BUTTON_SIZE.SMALL} | ||
| style={styles.pl2} | ||
| // Restores the 12px horizontal padding from the legacy implementation. | ||
| innerStyles={styles.ph3} | ||
|
Contributor
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. Same as in |
||
| onPress={() => removeEvent(reportID, action.reportActionID, event.id, events)} | ||
| > | ||
| <Text style={styles.buttonSmallText}>{translate('common.remove')}</Text> | ||
|
|
||
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.
AI sent me a message like this. Check if it really looks wrong; if so, we should fix it now during the migration.
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.
Nice find, we should look into it 👀