Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Inline price control create alert #8835
feat: Inline price control create alert #8835
Changes from all commits
2e75feb
37ec7f0
9cf5c7e
dfc9669
56592d6
0ddc6cf
dff9b55
add643e
4a41fef
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
tell me more 👀 we might need to be more proactive about this in other surfaces maybe, how did you find out?
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.
@gkartalis maybe you and @rajsam003 can find some time to pair on this? Looks like this argument, and the same comment, exists already in a few different screens.
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.
good point, yes def @rajsam003 feel free to drop some time next week on my cal
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.
As I said I referred other screens as well ( for eg. here, here. It seemed like its generally the case in lot of places. So I find it safer to use this prop.
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.
We looked into it with @rajsam003, turns out that there is an issue with react native modals as the comment suggests, not sure how we can resolve it and remove it but we confirmed that removing this line it breaks the flow (the set price screen modal cannot open).
so lets leave it as it is (MOPLAT will further investigate when migrating to
react-navigation
.