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

[$1000] Compose field's left border doesn't extend as the field expands #17424

Closed
6 tasks done
kavimuru opened this issue Apr 13, 2023 · 38 comments
Closed
6 tasks done

[$1000] Compose field's left border doesn't extend as the field expands #17424

kavimuru opened this issue Apr 13, 2023 · 38 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented Apr 13, 2023

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. Open chat and start typing
  2. Goto second line by pressing Shift + Return (Enter)
  3. Keep Typing and goto Next line by Pressing Shift + Return (Enter)
  4. Check line between + Button & type area (the left border of the compose field)

Expected Result:

Line Should extend along with text box size

Actual Result:

Line is not extending along with text box size

image

Workaround:

N/A - minimal impact

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: 1.3.0
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
Untitled
Screenshot 2023-04-13 at 2 41 58 PM

Screen.Recording.2023-04-13.at.2.41.22.PM.mov

Expensify/Expensify Issue URL:
Issue reported by: @corange-lab
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681377556974939

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019ff38561f983d47e
  • Upwork Job ID: 1646855304295243776
  • Last Price Increase: 2023-04-14
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 13, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

MelvinBot commented Apr 13, 2023

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

@conorpendergrast
Copy link
Contributor

@kavimuru this issue title seems... wrong 😆

@corange-lab
Copy link

@kavimuru this issue title seems... wrong 😆

@conorpendergrast i wrote that title and i don't have word for it so ..

@conorpendergrast
Copy link
Contributor

Oh hah, I just spotted that it's in the Slack thread about this 😄

@conorpendergrast
Copy link
Contributor

I'm going to re-title this to make it a little clearer, but I did enjoy that previous title. Reproduced

image

@conorpendergrast conorpendergrast changed the title Unconventional Aesthetics: Embracing Unique Design Elements Compose field's border doesn't extend as the field expands Apr 14, 2023
@conorpendergrast conorpendergrast changed the title Compose field's border doesn't extend as the field expands Compose field's left border doesn't extend as the field expands Apr 14, 2023
@conorpendergrast conorpendergrast added the External Added to denote the issue can be worked on by a contributor label Apr 14, 2023
@melvin-bot melvin-bot bot changed the title Compose field's left border doesn't extend as the field expands [$1000] Compose field's left border doesn't extend as the field expands Apr 14, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~019ff38561f983d47e

@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 14, 2023
@PrashantMangukiya
Copy link
Contributor

Proposal

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

When composer has multiline text at that time right border beside Plus icon does not expand vertically.

What is the root cause of that problem?

This is the parent view that consist all three button i.e. collapse, expand, and addAction

<View style={[
styles.dFlex, styles.flexColumn,
(this.state.isFullComposerAvailable || this.props.isComposerFullSize) ? styles.justifyContentBetween : styles.justifyContentEnd,
]}
>

Plus button shows right border via View having styles.chatItemAttachBorder as shown below:

<View style={styles.chatItemAttachBorder}>

chatItemAttachBorder consist below style:

App/src/styles/styles.js

Lines 1587 to 1590 in ab9b7d9

chatItemAttachBorder: {
borderRightColor: themeColors.border,
borderRightWidth: 1,
},

So during multiline plus button view does not expand vertically so line does not show. This is the root cause of the problem.

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

We have to do below changes to solve this

  1. Remove View from line 807 as shown below
<View style={styles.chatItemAttachBorder}>    // *** Remove this
   .... // Keep this content
</View>.     // *** Remove this
  1. Apply styles.chatItemAttachBorder to its parent View that starts at line 764 as shown below:
 <View style={[ 
     styles.dFlex, styles.flexColumn, 
     (this.state.isFullComposerAvailable || this.props.isComposerFullSize) ? styles.justifyContentBetween : styles.justifyContentEnd, 
    styles.chatItemAttachBorder,   // *** Add this
 ]} 
 > 

Note: We can rename styles.chatItemAttachBorder if wish, otherwise it is ok. It consist style for right border.

So this will solve the issue. It is working as expected in all platform. Sample video added in Results section below.

What alternative solutions did you explore? (Optional)

None

Results
Web.mov

@MelvinBot
Copy link

Triggered auto assignment to @aldo-expensify (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@AmjedNazzal
Copy link
Contributor

Proposal

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

Line in the composer doesn't extend as the user presses shift enter

What is the root cause of that problem?

This line is created by using a borderRight. this border is added with "styles.chatItemAttachBorder" but originally only added to the View element of the actionButton which does not wrap the entire element that will expand with the textArea

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

We can move the style to a higher level, that way the border is attached to the entire element and will expand properly

<AttachmentPicker>
    {({openPicker}) => (
        <>
            <View style={[
                styles.chatItemAttachBorder,
                styles.dFlex, styles.flexColumn,
                (this.state.isFullComposerAvailable || this.props.isComposerFullSize) ? styles.justifyContentBetween : styles.justifyContentEnd,
            ]}
            >

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@Pujan92
Copy link
Contributor

Pujan92 commented Apr 14, 2023

Proposal

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

Right border isn't applied to the composer entire inner area

What is the root cause of that problem?

Currently, right border is only given to the plus icon View so whenever multiple lines are rendered in the composer textarea, right border won't apply to the full available height.

<Tooltip text={this.props.translate('reportActionCompose.addAction')}>
<View style={styles.chatItemAttachBorder}>

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

Apply styles.chatItemAttachBorder to the parent View of these items instead of only applying it to the Plus icon view and remove not required View of Plus icon.

<View style={[
styles.dFlex, styles.flexColumn,
(this.state.isFullComposerAvailable || this.props.isComposerFullSize) ? styles.justifyContentBetween : styles.justifyContentEnd,
]}

@mollfpr
Copy link
Contributor

mollfpr commented Apr 14, 2023

Thank you guys for the proposals!

While everyone has the same correct solution and @PrashantMangukiya proposal is the first to post it right after the Help Wanted label. @aldo-expensify, we can go with @PrashantMangukiya's proposal!

Result Screenshot 2023-04-15 at 01 13 24 Screenshot 2023-04-15 at 01 13 17

🎀 👀 🎀 C+ reviewed!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 14, 2023
@MelvinBot
Copy link

📣 @PrashantMangukiya You have been assigned to this job by @aldo-expensify!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@PrashantMangukiya
Copy link
Contributor

@mollfpr @aldo-expensify Thank you. Accepted job on Upwork. I will prepare and submit PR within two hours asap.

@melvin-bot melvin-bot bot changed the title [$1000] Compose field's left border doesn't extend as the field expands [HOLD for payment 2023-04-26] [$1000] Compose field's left border doesn't extend as the field expands Apr 19, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Apr 19, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

MelvinBot commented Apr 19, 2023

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:

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: @corange-lab - $250
  • Contributor that fixed the issue: @PrashantMangukiya - $1000 base + 50% bonus = $1500.
  • Contributor+ that helped on the issue and/or PR: @mollfpr - $1000 base + 50% bonus = $1500.

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
Copy link

MelvinBot commented Apr 19, 2023

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:

  • [@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
  • [@mollfpr] 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:
  • [@mollfpr] 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:
  • [@conorpendergrast] Determine if we should create a regression test for this bug.
  • [@mollfpr] 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.
  • [@conorpendergrast] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/281076

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 26, 2023
@PrashantMangukiya
Copy link
Contributor

@conorpendergrast Ping for Upwork.

@corange-lab
Copy link

@conorpendergrast i am waiting for response on upwork.

@aldo-expensify
Copy link
Contributor

Conor is out of office and he will be back early next week to process your payments

@corange-lab
Copy link

@aldo-expensify Thanks for giving update.

@conorpendergrast conorpendergrast changed the title [HOLD for payment 2023-04-26] [$1000] Compose field's left border doesn't extend as the field expands [Pay day 2023-04-26] [$1000] Compose field's left border doesn't extend as the field expands Apr 27, 2023
@conorpendergrast
Copy link
Contributor

Thanks all, and appreciate you applying via Upwork - makes it a lot easier!

Assigned: April 14th 8:18pm
PR merged: April 17th 8:15pm
That's less than 3 business days, and so C and C+ are eligible for 50% bonus

@conorpendergrast
Copy link
Contributor

conorpendergrast commented Apr 27, 2023

  1. Invited and paid @PrashantMangukiya.
  2. Invited @mollfpr to the job in Upwork.
  3. @corange-lab I'm really sorry, I wasn't able to find your application on Upwork, and I haven't been able to find your Upwork profile! Can you either apply here, or reply with your Upwork profile URL and I'll invite you? Thanks!

@PrashantMangukiya
Copy link
Contributor

@conorpendergrast Received Thank you.

@corange-lab
Copy link

  • Invited and paid @PrashantMangukiya.
  • Invited @mollfpr to the job in Upwork.
  • @corange-lab I'm really sorry, I wasn't able to find your application on Upwork, and I haven't been able to find your Upwork profile! Can you either apply here, or reply with your Upwork profile URL and I'll invite you? Thanks!

@conorpendergrast
https://www.upwork.com/freelancers/~0182d5b08109999ffb
Thanks for response

@corange-lab
Copy link

@conorpendergrast Accepted Thanks.

@conorpendergrast
Copy link
Contributor

All paid now, thanks!

@conorpendergrast conorpendergrast removed the Awaiting Payment Auto-added when associated PR is deployed to production label Apr 28, 2023
@conorpendergrast conorpendergrast changed the title [Pay day 2023-04-26] [$1000] Compose field's left border doesn't extend as the field expands [$1000] Compose field's left border doesn't extend as the field expands Apr 28, 2023
@melvin-bot melvin-bot bot added the Overdue label May 1, 2023
@aldo-expensify
Copy link
Contributor

Friendly ping @mollfpr on completing: #17424 (comment)

@melvin-bot melvin-bot bot removed the Overdue label May 1, 2023
@mollfpr
Copy link
Contributor

mollfpr commented May 2, 2023

@aldo-expensify Sorry I miss this; I'll complete the checklist today 🙏

@mollfpr
Copy link
Contributor

mollfpr commented May 2, 2023

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] 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:

This issue has existed since the first implementation of the attachment button inside the composer #399. Then it got refactored here #13645, where it's not part of the button anymore.

So I don't think any of that PR is causing the bug, but now we see it as unsuitable.

[@mollfpr] 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:

The checklist is good for now!

@mollfpr
Copy link
Contributor

mollfpr commented May 2, 2023

[@mollfpr] 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.

For the regression test, I'll propose to have it under Compose box - Text Input. At Enter 3+ lines of text, and select/highlight the text, we can add Verify that the border after the action button is extended accordingly.

👍 or 👎?

@aldo-expensify
Copy link
Contributor

aldo-expensify commented May 2, 2023

That sounds perfect to me ^

@conorpendergrast
Copy link
Contributor

Sounds good to me too - I'll create that regression test

@conorpendergrast
Copy link
Contributor

Regression test created: https://github.com/Expensify/Expensify/issues/281076

Closing this out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

9 participants