adding some types, and do some refactor #133
Conversation
…empty-categories-on-search fix empty categories from briefly displaying when search begins
…atch-1 Update Request-the-Card.md
…ew-temporary-hidden
…tall Fix bundle install on macOS 15.1
…/feature/51906-card-list-search [Workspace feeds] Add search bar if cards assignment list has 8+ cards
…ject Check for empty object for the onboarding flow too
Workspace Feed - Use full cardsList instead of user cardList
sumo-slonik
force-pushed
the
@BartoszGrajdek/react-native-modal-refactor-Vkuba
branch
from
75bf192 to
e51ff8a
Compare
| @@ -210,22 +95,16 @@ function ReactNativeModal(incomingProps: ModalProps) { | |||
| return false; | |||
| }; | |||
|
|
|||
| const handleTransition = (type: 'open' | 'close', onFinish: () => void) => { | |||
| const handleTransition = (type: TransactionType, onFinish: () => void) => { | |||
…ve-unused-report-props [NO QA] chore: remove unused properties from Report type
…istance-rates fix: unify distance rates display
Co-authored-by: dukenv0307 <[email protected]>
Co-authored-by: dukenv0307 <[email protected]>
Co-authored-by: dukenv0307 <[email protected]>
…pressed-bg-color-on-menu-items Fix missing highlight bg color when pressing on menu item
…ls-report-field
fix: Inbox showing GBR when there’s a report with RBR
…ionButtons [CP Staging] Disable Search action button offline
…analytics-markers [NOQA] fix: performance analytics markers
[fix] Scan QAB is not displayed for offline created workspace
…action_button Hide add payment card button once a card is added
…ch-1 Update Configure-Quickbooks-Online.md
…n_moverbrworkspacechat
…Focus-popup-for-backdrop-click-and-back-button-click
[CP Staging] Revert "Show search action buttons"
fix selecting non-existing user in offline mode
| type OnOrientationChange = (orientation: NativeSyntheticEvent<any>) => void; | ||
|
|
||
| type OnSwipeCompleteParams = { | ||
| swipingDirection: Direction; | ||
| }; | ||
|
|
||
| type ModalProps = ViewProps & { | ||
| // Content inside the modal |
There was a problem hiding this comment.
FYI I'm pretty sure we should use /** Content */ comments instead of // Content - at least that's the convention we're sticking to in our E/App codebase
| useNativeDriverForBackdrop?: boolean; | ||
|
|
||
| animationIn?: string; // enum | ||
| // Enum for animation type when modal appears | ||
| animationIn?: string; |
There was a problem hiding this comment.
Can you add a TODO here and in animationOut, so that we remember to change this type to a string union later?
| animationInTiming?: number; | ||
| animationOut?: string; // enum | ||
|
|
||
| // Enum for animation type when modal disappears |
There was a problem hiding this comment.
Another thing is that this won't be an enum, since they are prohibited by eslint, I'd probably shorten this comment to something like "Animation type for dissapearing modal". Same goes for animationIn
| supportedOrientations?: Orientation[]; | ||
| }; | ||
|
|
||
| type GestureResponderEvent = NativeSyntheticEvent<NativeTouchEvent>; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| type AnimationEvent = (...args: any[]) => void; |
There was a problem hiding this comment.
(Optional for now) Can we somehow remove this any[]?
There was a problem hiding this comment.
that was the only one I had a problem with here and so far I haven't managed to replace it, but I'll take care of it
|
|
||
| const onMoveShouldSetPanResponder = (props: ModalProps, animEvt: MutableRefObject<AnimationEvent | null>, currentSwipingDirection: Direction | null, pan: Animated.ValueXY) => { | ||
| type RemainingModalProps = Omit< |
There was a problem hiding this comment.
(After we merge this PR) We'll probably need to figure out a better way to handle props later on, but that's fine for now.
| import {defaultProps} from './utils'; | ||
|
|
||
| type TransitionType = 'open' | 'close'; | ||
|
|
||
| function ReactNativeModal(incomingProps: ModalProps) { | ||
| const { |
There was a problem hiding this comment.
(After we merge this PR) If you have time, it would be great if you could look into how we're handling props. My idea was to not change anything compared to react-native-modal to avoid any regressions since we're passing a subset of incomingProps later to the returned Modal wrapper (Line 265). However, we should be able to make this code cleaner, given that we test it properly first.
|
Caution Gestures stopped working Most likely due to |
sumo-slonik
merged commit 1e78131
into
@BartoszGrajdek/react-native-modal-refactor
Explanation of Change
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop