-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix/30257: Room setting name different than header and LHN #31016
Fix/30257: Room setting name different than header and LHN #31016
Conversation
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2023-11-13.at.12.42.53.movAndroid: mWeb ChromeScreen.Recording.2023-11-13.at.12.47.22.moviOS: NativeScreen.Recording.2023-11-13.at.12.40.36.moviOS: mWeb SafariScreen.Recording.2023-11-13.at.12.45.45.movMacOS: Chrome / SafariScreen.Recording.2023-11-13.at.12.49.19.movMacOS: DesktopScreen.Recording.2023-11-13.at.12.51.24.mov |
src/libs/ReportUtils.js
Outdated
*/ | ||
function getAllPersonalDetails() { | ||
return allPersonalDetails; | ||
} |
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'm not sure about this - wouldn't be better to connect to onyx inside the GroupChatUtils
file to retrieve this value directly?
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.
just updated
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.
one comment left
@burczu updated. Please help review |
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.
LGTM
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/Li357 in version: 1.3.99-0 🚀
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.99-0 🚀
|
🚀 Deployed to staging by https://github.com/Li357 in version: 1.4.0-0 🚀
|
Details
Fixed Issues
$ #30257
PROPOSAL: #30257 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.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.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Screencast.from.08-11-2023.11.34.33.webm
Android: mWeb Chrome
Screencast.from.08-11-2023.10.28.31.webm
iOS: Native
Screencast.from.08-11-2023.09.34.18.webm
iOS: mWeb Safari
Screencast.from.08-11-2023.10.12.35.webm
MacOS: Chrome / Safari
Screencast.from.08-11-2023.00.32.01.webm
MacOS: Desktop
Screencast.from.08-11-2023.09.04.57.webm