Skip to content
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

[hold for payment 2024-04-29] [$500] Live markdown for money request description #39192

Closed
6 tasks done
roryabraham opened this issue Mar 28, 2024 · 50 comments
Closed
6 tasks done
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review

Comments

@roryabraham
Copy link
Contributor

roryabraham commented Mar 28, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1711641837108169

Action Performed:

  1. Press on the global create button
  2. Press Request Money -> Manual -> enter any amount -> choose any participants
  3. Press on the description field
  4. type markdown text into the description field

Note: There are more permutations of this, the intention with this issue is that it should capture all money request description fields, including:

  • 1:1 requests, splits, and send money
  • different types of requests such as manual v.s. scan v.s. distance

Expected Result:

The markdown styling should render in real-time, as it does for the main chat composer.

Actual Result:

No live markdown formatting.

Workaround:

n/a

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0147562f8d45f3f241
  • Upwork Job ID: 1773388950435934208
  • Last Price Increase: 2024-04-11
  • Automatic offers:
    • alitoshmatov | Reviewer | 0
    • ShridharGoel | Contributor | 0
@roryabraham roryabraham added External Added to denote the issue can be worked on by a contributor Daily KSv2 NewFeature Something to build that is a new item. labels Mar 28, 2024
@melvin-bot melvin-bot bot changed the title Live markdown for money request description [$500] Live markdown for money request description Mar 28, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 28, 2024
@roryabraham roryabraham moved this to Todo in Live Markdown Mar 28, 2024
Copy link

melvin-bot bot commented Mar 28, 2024

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

Copy link

melvin-bot bot commented Mar 28, 2024

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Mar 28, 2024
Copy link

melvin-bot bot commented Mar 28, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

@roryabraham
Copy link
Contributor Author

accidentally included the upwork tag from the previous issue in a copy/pasta situation. Going to try re-applying External to see if we can get new upwork jobs created

@roryabraham roryabraham added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Mar 28, 2024
Copy link

melvin-bot bot commented Mar 28, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0147562f8d45f3f241

Copy link

melvin-bot bot commented Mar 28, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Mar 28, 2024
@ShridharGoel
Copy link
Contributor

ShridharGoel commented Mar 28, 2024

Proposal

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

Live markdown support is needed for money request description.

What is the root cause of that problem?

New feature. Currently InputWrapperWithRef with TextInput is being used and internally BaseTextInput uses RNTextInput.

<InputWrapperWithRef
InputComponent={TextInput}
inputID={INPUT_IDS.MONEY_REQUEST_COMMENT}
name={INPUT_IDS.MONEY_REQUEST_COMMENT}
defaultValue={currentDescription}
label={translate('moneyRequestConfirmationList.whatsItFor')}
accessibilityLabel={translate('moneyRequestConfirmationList.whatsItFor')}
role={CONST.ROLE.PRESENTATION}
ref={(el) => {
if (!el) {
return;
}
inputRef.current = el;
updateMultilineInputRange(inputRef.current);
}}
autoGrowHeight
containerStyles={[styles.autoGrowHeightMultilineInput]}
shouldSubmitForm
/>
</View>

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

Use RNMarkdownTextInput as being used in composer.

Same field is used for different types of requests like send money, 1:1 requests, split bills etc. Also, it will update in all of manual, scan and distance.

We can pass a new boolean prop markdownEnabled and if it is true, then RNMarkdownTextInput should be used in BaseTextInput.

This is already done here: #39519

After the above PR is merged, we can pass markdownEnabled via IOURequestStepDescription.

<InputWrapperWithRef
    InputComponent={TextInput}
    inputID={INPUT_IDS.MONEY_REQUEST_COMMENT}
    name={INPUT_IDS.MONEY_REQUEST_COMMENT}
    defaultValue={currentDescription}
    label={translate('moneyRequestConfirmationList.whatsItFor')}
    accessibilityLabel={translate('moneyRequestConfirmationList.whatsItFor')}
    role={CONST.ROLE.PRESENTATION}
    ref={(el) => {
        if (!el) {
            return;
        }
        inputRef.current = el;
        updateMultilineInputRange(inputRef.current);
    }}
    autoGrowHeight
    containerStyles={[styles.autoGrowHeightMultilineInput]}
    shouldSubmitForm
    markdownEnabled
/>

Testing plan

Check the working of the live markdown feature at all places where IOURequestStepDescription is used, like request money including manual, scan, distance, 1:1 DM, splits, send money etc. This would include testing things like blockquote,
code fence, inline code block, email, links, heading, italics, bold and strikethrough.

Rollout plan

We have two options:

  1. Launch for all avenues.
  2. We can first do a rollout only for a single avenue (like only split bills). Once we find that it's working fine in production, then we can enable for all places. This can be easily handled by having a new prop called shouldEnableLiveMarkdown. If this is true, then live markdown would work for that feature. Later, we can remove this prop and enable for all descriptions.
Screen.Recording.2024-04-05.at.5.39.53.PM.mov

@allgandalf
Copy link
Contributor

Proposal

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

Live markdown for money request description

What is the root cause of that problem?

Inside of InputWrapperWithRef we pass TextInput to InputComponent:

<InputWrapperWithRef
InputComponent={TextInput}

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

We should instead pass InputComponent={RNMarkdownTextInput}

What alternative solutions did you explore? (Optional)

N/A

@allgandalf
Copy link
Contributor

Note to my proposal: Additional style changes would also be required !

@joekaufmanexpensify
Copy link
Contributor

proposals pending review

@melvin-bot melvin-bot bot added the Overdue label Apr 1, 2024
@allgandalf
Copy link
Contributor

friendly bump to @alitoshmatov for review :)

@joekaufmanexpensify
Copy link
Contributor

Proposals still pending review

@melvin-bot melvin-bot bot removed the Overdue label Apr 1, 2024
@roryabraham
Copy link
Contributor Author

for context - there was some suggestion(s) from SWM folks that perhaps we should wait for live markdown on web to go to prod before working on this.

@roryabraham
Copy link
Contributor Author

#39519 is merged so we should feel free to proceed with the PR now

@ShridharGoel
Copy link
Contributor

Sure, will open a PR soon.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Apr 15, 2024
@joekaufmanexpensify
Copy link
Contributor

Great. TY!

@roryabraham
Copy link
Contributor Author

#40224 is up for review

@roryabraham roryabraham self-assigned this Apr 15, 2024
@joekaufmanexpensify
Copy link
Contributor

automation was not working, but PR is on production for 7 days. So going to prep for payment today!

@joekaufmanexpensify joekaufmanexpensify added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Apr 29, 2024
@joekaufmanexpensify joekaufmanexpensify changed the title [$500] Live markdown for money request description [hold for payment 2024-04-29] [$500] Live markdown for money request description Apr 29, 2024
@joekaufmanexpensify
Copy link
Contributor

This issue was opened prior to April 4th, so we need to issue the following payments:

@joekaufmanexpensify
Copy link
Contributor

@ShridharGoel please accept our offer in upwork so we can pay you

@joekaufmanexpensify
Copy link
Contributor

@alitoshmatov $500 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Bumped in Slack

@joekaufmanexpensify
Copy link
Contributor

Still pending @ShridharGoel accepting upwork offer

@joekaufmanexpensify joekaufmanexpensify added Weekly KSv2 and removed Daily KSv2 labels May 1, 2024
@ShridharGoel
Copy link
Contributor

@joekaufmanexpensify I've accepted it now, thanks.

@joekaufmanexpensify joekaufmanexpensify added Daily KSv2 and removed Weekly KSv2 labels May 2, 2024
@joekaufmanexpensify
Copy link
Contributor

TY! will pay today

@joekaufmanexpensify
Copy link
Contributor

@ShridharGoel $500 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Upwork job closed.

@joekaufmanexpensify
Copy link
Contributor

All set, TY everyone!

@thienlnam thienlnam moved this from Todo to Done in Live Markdown Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

5 participants