[No QA] Add a header to FullPageNotFoundView#10155
Merged
Merged
Conversation
neil-marcellini
requested changes
Jul 29, 2022
neil-marcellini
left a comment
Contributor
There was a problem hiding this comment.
Thanks, looks great! One comment about the close / back behavior.
aldo-expensify
previously requested changes
Jul 29, 2022
aldo-expensify
left a comment
Contributor
There was a problem hiding this comment.
Looking good, just a comment on simplifying the template (if possible)
neil-marcellini
approved these changes
Aug 1, 2022
Changes addressed and OOO
Contributor
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
95 tasks
Contributor
|
🚀 Deployed to staging by @neil-marcellini in version: 1.1.87-0 🚀
|
Contributor
|
🚀 Deployed to production by @yuwenmemon in version: 1.1.87-9 🚀
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
cc @neil-marcellini
Details
Adds a header with buttons that dismiss the page to the FullPageNotFoundView to navigate the user back to the chat/reports view.
Fixed Issues
$ #10082
Tests
ScreenWrapperin InitialSettingsPage.js with the newFullPageNotFoundViewshouldShowprop tofalseor omit it.shouldShowprop totruePR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followed/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
None
Screenshots
Web
Mobile Web
Desktop
iOS
Android