[HOLD for payment 2023-04-26] [$1000] Storybook - Errors appear in the JS console for Forms and PopoverMenu

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Go to Form
2. Go to PopoverMenu > click "Add payment Methods" > click on any option

Expected Result:

Verify that no errors appear in the JS console and components (Form/PopoverMenu) renders without a problem.

Actual Result:

Errors appear in the JS console and components (Form/PopoverMenu) does not render due to an error.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number:
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @fedirjh
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1680745076761309

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0191bc581b220c1229
• Upwork Job ID: 1645418768840740864
• Last Price Increase: 2023-04-10

[MelvinBot]

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[situchan]

I'd like to fix this issue (without any compensation) as one of regressions is from the issue I worked on.

Edit: I confirmed that storybook issue existed before PR I worked on so not a regression.
I would say #13580 is only the offending PR which caused regression.

From slack convo:

The Form issue is a regression from #16332 and #13580

[joekaufmanexpensify]

Reproduced.

[joekaufmanexpensify]

Thanks @situchan ! We'll just finishing triaging this, but I imagine we'll end up taking you up on your offer there.

[MelvinBot]

Triggered auto assignment to @cristipaval (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[joekaufmanexpensify]

@cristipaval LMK if you agree with having @situchan fix this!

[MelvinBot]

@cristipaval, @joekaufmanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MelvinBot]

@cristipaval, @joekaufmanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[cristipaval]

@joekaufmanexpensify yes I do agree, but I think we still need a proposal and a C+ to review the proposal and the PR.

[joekaufmanexpensify]

Sounds good! @situchan could you please share your proposal?

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~0191bc581b220c1229

[MelvinBot]

Current assignee @joekaufmanexpensify is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

[MelvinBot]

Current assignee @cristipaval is eligible for the External assigner, not assigning anyone new.

[ahmedGaber93]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Storybook - Errors appear in the JS console for Forms and PopoverMenu
null is not an object (evaluating 'insets.top')

What is the root cause of that problem?

Form

missed SafeAreaProvider, in file .storybook/preview.js we need to add context provider SafeAreaProvider

PopoverMenu

document.querySelector('meta[name=theme-color]') return null because missed <meta name="theme-color" content=""> in storybook files
<meta name="theme-color" content=""> meta found in web/index.html

What changes do you think we should make in order to solve the problem?

Form

in file .storybook/preview.js we need to add SafeAreaProvider

PopoverMenu

in file .storybook/preview.js define <meta name="theme-color" content="">

import getPlatform from '../src/libs/getPlatform';
import {SafeAreaProvider} from 'react-native-safe-area-context';

const decorators = [
    (Story) => {
        React.useEffect(() => {
            const isWeb = _.contains([CONST.PLATFORM.WEB, CONST.PLATFORM.DESKTOP], getPlatform());
            if (isWeb && !document.querySelector('meta[name=theme-color]')) {
                const meta = document.createElement('meta');
                meta.setAttribute('name', 'theme-color');
                meta.setAttribute('content', '');
                document.getElementsByTagName('head')[0].appendChild(meta);
            }
        }, []);

        return (
            <ComposeProviders
                components={[
                    OnyxProvider,
                    SafeAreaProvider,
                    LocaleContextProvider,
                    HTMLEngineProvider,
                ]}
            >
                <Story />
            </ComposeProviders>
        );
    },
];

What alternative solutions did you explore? (Optional)

[fedirjh]

Go to PopoverMenu > click "Add payment Methods" > click on any option

@ahmedGaber93 We also have PopoverMenu that needs to be fixed. Your proposal only cover the Form component, Could you please update your proposal to address PopoverMenu as well?

[situchan]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Error appears in Storybook Form and PopoverMenu / Add payment Methods

What is the root cause of that problem?

• Form is using SafeAreaConsumer but root view is not wrapped with SafeAreaProvider so insets callback value doesn't exist
• We set theme-color to set status bar color on mWeb using document.querySelector. On web, this is defined as meta tag in web/index.html but not for storybook so value is null and null.content causes error

What changes do you think we should make in order to solve the problem?

1. add SafeAreaProvider here
2. add null safety check here and here

[MelvinBot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[MelvinBot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.1-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix storybook errors on Forms and PopoverMenu #17443

If no regressions arise, payment will be issued on 2023-04-26. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[MelvinBot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mananjadhav] The PR that introduced the bug has been identified. Link to the PR: Referenced here: Remove safe area padding for some screens #13580
• [@mananjadhav] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mananjadhav] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: [HOLD for payment 2023-04-26] [$1000] Storybook - Errors appear in the JS console for Forms and PopoverMenu #17092 (comment)
• [@joekaufmanexpensify] Determine if we should create a regression test for this bug.
• [@mananjadhav] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@joekaufmanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[joekaufmanexpensify]

Okay, getting ready for issuing payment in a few days. @situchan was assigned to this issue on 2023-04-13. The PR was merged on 2023-04-17. The 15th, and 16th were weekend days though, so this PR only took 2 business days to merge and qualifies for a speed bonus.

With that in mind, we need to pay:

• @situchan $1,500 ($1,000 will be contract amount, and $500 a bonus during payment).
• @mananjadhav $1,500 ($1,000 will be contract amount, and $500 a bonus during payment).
• @fedirjh $250

[joekaufmanexpensify]

@situchan offer sent for $1,000!

[joekaufmanexpensify]

@mananjadhav offer sent for $1,000!

[joekaufmanexpensify]

@fedirjh offer sent for $250!

[joekaufmanexpensify]

@mananjadhav @cristipaval I feel like no regression test is needed here because this isn't a user facing error. Curious if y'all agree?

[cristipaval]

yes I do agree, this can't be tested by Applause team so it doesn't make sense to add a regression test.

[joekaufmanexpensify]

Awesome. Just checked off those steps.

[joekaufmanexpensify]

@mananjadhav , mind taking care of the remaining BZ steps? That's all we need to do before we issue payment here.

[mananjadhav]

@joekaufmanexpensify @cristipaval the offending PR is linked in the earlier chats #13580 and I don't think this needs a regression test. Could you let me know which other steps are pending?

[cristipaval]

I think we can close this issue if the payments are done

[mananjadhav]

Payments are pending for this one.

[joekaufmanexpensify]

A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Is there not anything we need to discuss to catch this type of bug sooner? That's cool, if not. But in general it's good idea to note on the GH issue so we can make sure we're completing all of the Bug Zero checklist options, or at least acknowledging that we think one of the steps is not necessary, and why.

[mananjadhav]

If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.

There is an item in the checklist related to Storybook. With the linked PR, the issue was we had too many test cases to test as the change had an impact across the App.

[joekaufmanexpensify]

Got it, makes sense. Added that explanation to the above section of the BZ checklist!

[mananjadhav]

Thanks for the follow up @joekaufmanexpensify . Will you be able to now process the payout?

[joekaufmanexpensify]

Yep, we're all set, going to do it now!

[joekaufmanexpensify]

@situchan $1,500 sent and contract ended!

[joekaufmanexpensify]

@mananjadhav $1,500 sent and contract ended!

[joekaufmanexpensify]

@fedirjh $250 sent and contract ended!

[joekaufmanexpensify]

Upwork job closed.

[joekaufmanexpensify]

Bug is fixed, BZ checklist complete, and all payment issued. Closing as this is all set. TY everyone!