-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Add not found page to report RHN workspace detail pages #14744
Conversation
cc @shawnborton |
@eVoloshchak @neil-marcellini One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
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.
Tests well with user B on mobile web. The code looks good to me. I'll let @eVoloshchak do a thorough testing next.
Looks good to me! I think we can eventually follow up and replace this with an illustration if we wanted to, but I think what you have works for now. |
Reviewer Checklist
Screenshots/VideosWebScreen.Recording.2023-02-07.at.19.14.07.movMobile Web - ChromeScreen.Recording.2023-02-07.at.19.39.56.movMobile Web - SafariScreen.Recording.2023-02-07.at.19.27.41.movDesktopScreen.Recording.2023-02-07.at.19.20.02.moviOSScreen.Recording.2023-02-07.at.19.22.04.movAndroidScreen.Recording.2023-02-07.at.19.36.44.mov |
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.
Tests well!
UPD:
This isn't caused by this PR, but I noticed that the user is still able to view other members' info even after the user has been deleted from the workspace.
For instance, when entering new.expensify.com/r/1143940934986173/participants
, I get the not found page, but if I go to new.expensify.com/r/1143940934986173/participants/[email protected]
, I can still view the user's info. Is that the intended behavior?
@eVoloshchak I think that should be ok. Even if you are not a member of the workspace, you could still create a chat with that person by searching their email and you'd have access to their details that way. So unless we restrict creating chats to contacts only (which I don't think we'll do), you'd always have access to participant details. |
🚀 Deployed to staging by https://github.com/luacmartins in version: 1.2.67-1 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.2.67-7 🚀
|
Details
Adds a not found page to the report RHN pages
Fixed Issues
$ #14683
Tests
+ > New workspace
+ > Workspace > Manage members > Invite
#announce > details > members
Not found
page in the RHN#announce > details
and#announce > details > settings
Offline tests
Regular regression tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.Screenshots/Videos
Web
web.mov
Mobile Web - Chrome
chrome.mov
Mobile Web - Safari
safari.mov
Desktop
desktop.mov
iOS
ios.mov
Android
android.mov