-
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
E2E: Mobile Drafts tests cases #8469
base: main
Are you sure you want to change the base?
Conversation
yasserfaraazkhan
commented
Jan 10, 2025
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.
Minor comment. LGTM 🚀
Co-authored-by: Rajat Dabade <[email protected]>
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.
Great work! Just one thing to verify before approving.
@@ -211,6 +212,7 @@ const PostPriorityPicker = ({ | |||
type='toggle' | |||
selected={data.persistent_notifications} | |||
descriptionNumberOfLines={2} | |||
value='persistent_notifications' |
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.
I was checking the implementation of how this value is used, and something feels really off. Can you verify if this doesn't break anything? The test I would do is:
- Turn on persistent notifications
- Turn them off again (verify they get visually turned off)
- Create the post and verify the post was created without persistent notifications.