-
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
changes for wording for use plus button #23248
changes for wording for use plus button #23248
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
@luacmartins Please 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] |
I have read the CLA Document and I hereby sign the CLA |
@thesahindia I am not able to navigate to a Workspace chat like here to verify the changes. When i create a Workspace only 2 new rooms created # announcements and # admins, workspace chat is not in LHN |
I'll be reviewing this |
@ishpaul777 for that you'll need to do the following step
|
@rushatgabhane still not getting workspace only getting the rooms. |
@rushatgabhane is there something i missed ? |
748e871
to
d6b30ee
Compare
d6b30ee
to
4e437b7
Compare
@ishpaul777 maybe invite users to your workspace |
I think there's a beta for this. @ishpaul777 what email are you using to test? |
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
@luacmartins |
Not sure why Melvin assigned me to this issue when @amyevans is the CME 😕 |
Ok, added you to the beta. Try again in a few minutes to an hour. |
@ishpaul777 lint is failing. Try running |
will do it! Btw @luacmartins i dont know why my first is not signed. is there a way i can sign it now? |
tested in offline mode @rushatgabhane Can you review? |
Hey @rushatgabhane @luacmartins, I saw @ishpaul777's message on slack and since they don't have a mac and scaleway's mac services are on hold. I volunteered to help with test proofs if you two agree it's okay this one time. |
@Nikhil-Vats thanks for the help! yeah that works. you can post them as a comment here |
i added the screenshots @rushatgabhane except for android and mobile chrome was not able to build the app for them on scaleaway mac instance. |
I am done from side @rushatgabhane. I tried reaching out on slack @Nikhil-Vats but he is not available. I am not able to get the remaing screenshots, also i have seen multiple closed PR's without screenshots, please guide me what should i do. |
@Nikhil-Vats Thank you so much! ✨ @rushatgabhane PR is ready! |
@rushatgabhane @amyevans can you please update on when this should be final reviewed and merged ? |
Reviewer Checklist
Screenshots/Videos |
@rushatgabhane if you have any update or concerns with the PR, please let me know? |
thanks for completing the checklist @ishpaul777 I'll try to prioritise the PR |
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
🎯 @rushatgabhane, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #23495. |
✋ 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/amyevans in version: 1.3.45-0 🚀
|
@rushatgabhane @luacmartins Can you pls confirm we dont need to QA this? Steps are missing |
@mvtglobally QA steps -
|
Thanks for the QA steps @rushatgabhane! |
🚀 Deployed to production by https://github.com/mountiny in version: 1.3.45-7 🚀
|
@luacmartins @amyevans Can I get a written confirmation from one of you that @rushatgabhane should be paid out $1,000 for his work helping review this PR? |
@JmillsExpensify He's being paid $1500 per the linked issue, he took over as C+ there |
Details
Fixed Issues
$ #19828
PROPOSAL
Tests
ref video- #19828 (comment)
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
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android