-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Refactor CloseAccountPage to functional component #19328
Merged
Merged
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
014d239
refactor to functional component
bondydaa db0d53e
Merge branch 'main' of github.com:Expensify/App into bondy-refactor-c…
bondydaa bd80ad9
remove unused method
bondydaa 86c0b3f
dont use useCallback here as its most likely not needed
bondydaa df15ca0
clean up imports
bondydaa ac211e3
Merge branch 'main' of github.com:Expensify/App into bondy-refactor-c…
bondydaa 270b71c
export default and clean up usage
bondydaa 5d050a0
fix default error key as defined by Onyx::handleException in web-e
bondydaa ba1c8fb
Merge branch 'main' of github.com:Expensify/App into bondy-refactor-c…
bondydaa ecbef94
Revert "remove unused method"
bondydaa 4779d8e
Revert "export default and clean up usage"
bondydaa cce3f22
Revert "clean up imports"
bondydaa e25a3b1
copy functionality based on lifecycle methods to not change behavior …
bondydaa 5da45fa
fix default type to stop console error
bondydaa 477631e
remove setting default data on component mount, its not 100% necessary
bondydaa 3830a3e
fix linter
bondydaa 633535b
Merge branch 'main' of github.com:Expensify/App into bondy-refactor-c…
bondydaa b9069f7
fix styles with prettier
bondydaa d5d59c2
prevent execution of hook on every re-render by passing empty depeden…
bondydaa a3ffe91
Merge branch 'main' of github.com:Expensify/App into bondy-refactor-c…
bondydaa 7bcb7a9
initial fix of merge conflicts, doesnt include all changes yet though
bondydaa d71f54b
apply changes from commit 38e49c372
bondydaa 57a33a4
pull in change from commit b2dda336
bondydaa 3c806f1
pull in changes from commit 1b36c75b17
bondydaa 7bd01cb
pull in changes from commit 3b5e279fe7
bondydaa 15f6982
pull in changes from commit 1e456f6788d1d
bondydaa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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 think we should clear the errors as before because the next time I open the page, the loader is shown on the Close Account button.
Steps.
Might be unrelated to this change but exists.
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.
hmm you might be right, I noticed at one point in testing but then it went away after I'd reloaded and then I couldn't reproduce. Let me try again.
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.
Though don't think this method would actually stop the loader from showing since I think that is controlled by the
isLoading
key that's defined here:App/src/CONST.js
Line 730 in 63f314c
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.
The more I noodle on this I think maybe we shouldn't be trying to address this here?
Really these props are all reacted-to by the
Form
component, this component is sort of just attempting to pass that data down along to it.The only way to really solve it is to use the
useEffect
/ lifecycle methods that were being used but those don't really make sense to add here since our component doesn't actually depend on them.If we were to re-implement the lifecycle methods here we'd need
useEffect
which is just going to set some onyx data to the default because that's what the<Form>
component will react to.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 tried again and now this issue is not reproducible as you said.
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.
But I think that these errors are backend errors that we were clearing earlier. But I am not sure how to create a backend error so I can't test that.