Closed jayeshmangwani closed 1 week ago
@suneox 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]
@DylanDylann Please help review this PR
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and not onIconClick
).myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components using Avatar
have been tested & I retested again)/** comment above it */
this
properly so there are no scoping issues (i.e. for onClick={this.submit}
the method this.submit
should be bound to this
in the constructor)this
are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this);
if this.submit
is never passed to a component event handler like onClick
)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG
)Avatar
is modified, I verified that Avatar
is working as expected in all cases)Design
label and/or tagged @Expensify/design
so the design team can review the changes.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 the Test
steps.Also FYI @jayeshmangwani I added $
before ALL GH issues in your PR description, not just the first one π
@MariaHCD @Beamanator updated the Explanation of Change section
and added a comment on the page explaining why we made this change. The comment is quite detailed, so let me know if you'd like me to revise or shorten it.
I added $ before ALL GH issues in your PR description, not just the first one π
Thanks for doing this! I'll make sure to add $
to all the issues in the future."
:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.
π Cherry-picked to staging by https://github.com/Beamanator in version: 9.0.84-3 π
platform | result |
---|---|
π€ android π€ | success β |
π₯ desktop π₯ | success β |
π iOS π | success β |
πΈ web πΈ | success β |
π€π android HybridApp π€π | success β |
ππ iOS HybridApp ππ | success β |
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.
π Deployed to production by https://github.com/mountiny in version: 9.0.84-7 π
platform | result |
---|---|
π€ android π€ | true β |
π₯ desktop π₯ | failure β |
π iOS π | success β |
πΈ web πΈ | success β |
π€π android HybridApp π€π | failure β |
ππ iOS HybridApp ππ | success β |
π Deployed to production by https://github.com/mountiny in version: 9.0.84-7 π
platform | result |
---|---|
π€ android π€ | true β |
π₯ desktop π₯ | success β |
π iOS π | success β |
πΈ web πΈ | success β |
π€π android HybridApp π€π | failure β |
ππ iOS HybridApp ππ | success β |
Explanation of Change
In this PR, we implemented two main changes:
UpgradeIntro Component Update Previously, the
GenericFeaturesView
was shown if thefeature
wasnull
or there was nopolicyID
. However, this approach failed for users accessing the "Categorize" option. When a user selects "Categorize" from the Self-DM whisper, theisCategorizing
flag is set to true. In such scenarios, a separate Categories upgrade UI needs to be displayed. To address this, we added a condition to ensure thatisCategorizing
is false before checking thepolicyID
.SubscriptionPlanCard PressableWithFeedback Refactoring We removed the parent
View
around thePressableWithFeedback
for SubscriptionPlanCard component. This ensures that the PressableWithFeedback child has no untouchable area, allowing the entire card to act as a PressableWithFeedback.Fixed Issues
$ https://github.com/Expensify/App/issues/55110 $ https://github.com/Expensify/App/issues/55111 $ https://github.com/Expensify/App/issues/55112 $ https://github.com/Expensify/App/issues/55113 PROPOSAL:
Tests
Test 1
Test 2
Test 3
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]." Same as Tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.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
https://github.com/user-attachments/assets/45b6fb7a-5aed-4a1b-9c44-b1704448e97bAndroid: mWeb Chrome
https://github.com/user-attachments/assets/060acb57-0ecc-4d59-b692-dda18ff416b1iOS: Native
https://github.com/user-attachments/assets/50a909da-2a1a-43ce-8551-443120cbad4fiOS: mWeb Safari
https://github.com/user-attachments/assets/d282edb1-1d54-4717-ac6d-b7d1fac5d1c5MacOS: Chrome / Safari
https://github.com/user-attachments/assets/e15fa916-a84e-4984-8a20-60105770361fMacOS: Desktop
https://github.com/user-attachments/assets/91f334bd-2bb5-4962-82ff-6bcbbb2cfb3a