-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Swipable delete for Schedule post #8627
base: empty-state-scheduled-post
Are you sure you want to change the base?
Swipable delete for Schedule post #8627
Conversation
Changes look fine. Needs tests. |
736f6f9
to
d1c04a9
Compare
92506d1
to
1bdb012
Compare
d1c04a9
to
a6e995f
Compare
defaultMessage={'Delete draft'} | ||
style={styles1.deleteText} | ||
/> | ||
{draftType === DRAFT_TYPE_DRAFT ? ( |
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.
Missing tests for this new behavior?
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.
Added test
1bdb012
to
39e11d0
Compare
a6e995f
to
fb548cb
Compare
fb548cb
to
eb0e95b
Compare
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.
One minor thing to fix in the new tests.
DeviceEventEmitter.emit(Events.DRAFT_SWIPEABLE, mockDraft.id); | ||
|
||
expect(emitSpy).toHaveBeenCalledWith(Events.DRAFT_SWIPEABLE, 'draft-id-1'); |
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.
This is only testing that you are calling the function that you did call in the test. This is testing nothing.
@@ -90,4 +90,3 @@ const ScheduledPostTooltip = ({onClose}: Props) => { | |||
}; | |||
|
|||
export default ScheduledPostTooltip; | |||
|
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.
Is this change intended?
Summary
Ticket Link
Checklist
E2E iOS tests for PR
.Device Information
This PR was tested on:
Screenshots
Release Note