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-07-24] [$125] Transfer owner - "Continue" button is missing bottom margin #44825

Closed
4 of 6 tasks
izarutskaya opened this issue Jul 4, 2024 · 37 comments
Closed
4 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production 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

@izarutskaya
Copy link

izarutskaya commented Jul 4, 2024

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


Version Number: 9.0.4-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: #44712
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

  • User is invited to a workspace.
  • User is also promoted to admin.
  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Members.
  3. Click on the owner.
  4. Click Transfer owner.

Expected Result:

"Continue" button on "Duplicate subscription alert" page will have bottom margin.

Actual Result:

"Continue" button on "Duplicate subscription alert" page does not have bottom margin.

Workaround:

Unknown

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

Bug6532565_1720050037604.20240704_073840.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01851c277cf7d0a07b
  • Upwork Job ID: 1811060685379521358
  • Last Price Increase: 2024-07-11
Issue OwnerCurrent Issue Owner: @miljakljajic
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 4, 2024
Copy link

melvin-bot bot commented Jul 4, 2024

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@izarutskaya
Copy link
Author

We think this issue might be related to the #collect project.

@etCoderDysto
Copy link
Contributor

Proposal

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

"Continue" button is missing bottom margin

What is the root cause of that problem?

In WorkspaceOwnerChangeCheck we are not adding bottom margin to the Button component

<Button
success
large
onPress={confirm}
text={displayTexts.buttonText}

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

  1. Wrap the Button in View component
  2. Add styles.pb5 to the View component
<View style={styles.pb5}>
                <Button...

What alternative solutions did you explore? (Optional)

@Krishna2323
Copy link
Contributor

Proposal

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

Transfer owner - "Continue" button is missing bottom margin

What is the root cause of that problem?

Padding/Margin bottom isn't added when error !== CONST.POLICY.OWNERSHIP_ERRORS.NO_BILLING_CARD is true.

<View style={[styles.containerWithSpaceBetween, error !== CONST.POLICY.OWNERSHIP_ERRORS.NO_BILLING_CARD ? styles.ph5 : styles.ph0, styles.pb0]}>

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

Add Padding/Margin bottom when error !== CONST.POLICY.OWNERSHIP_ERRORS.NO_BILLING_CARD is true just like we add horizontal padding.

What alternative solutions did you explore? (Optional)

@nkdengineer
Copy link
Contributor

Proposal

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

Transfer owner - "Continue" button is missing bottom margin

What is the root cause of that problem?

<Button
success
large
onPress={confirm}
text={displayTexts.buttonText}
/>

We don't add bottom margin to the button

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

We implemented FixedFooter Component to align this bug on other pages

<FixedFooter style={[styles.mtAuto, styles.pt5]}>
                <Button
                    success
                    large
                    onPress={confirm}
                    text={displayTexts.buttonText}
                />
</FixedFooter>

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Overdue label Jul 8, 2024
Copy link

melvin-bot bot commented Jul 9, 2024

@miljakljajic Eep! 4 days overdue now. Issues have feelings too...

@miljakljajic miljakljajic added the External Added to denote the issue can be worked on by a contributor label Jul 10, 2024
@melvin-bot melvin-bot bot changed the title Transfer owner - "Continue" button is missing bottom margin [$250] Transfer owner - "Continue" button is missing bottom margin Jul 10, 2024
Copy link

melvin-bot bot commented Jul 10, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01851c277cf7d0a07b

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 10, 2024
Copy link

melvin-bot bot commented Jul 10, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Jul 10, 2024
@divyeshgajera1
Copy link

App/src/pages/workspace/members/WorkspaceOwnerChangeCheck.tsx

  • Add wrapper view in this screen top of the button and add marginBottom as per need

Copy link

melvin-bot bot commented Jul 10, 2024

📣 @divyeshgajera1! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@mehedy201
Copy link

``Please go to this WorkspaceMemberDetailsPage.tsx file. line no: 182 and 192 and try with this code. in style={[styles.mv5]}

Copy link

melvin-bot bot commented Jul 10, 2024

📣 @mehedy201! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@Daniel-Kline-1
Copy link

Daniel-Kline-1 commented Jul 10, 2024

Problem:
The button is below where it should be, on the bottom of the page

There are a few things that I think could be causing the issue.

It looks like there is an improperly written css value. I would want to look at styles.mtAuto and styles.pt5

A few things I will look for,

  • are you using flex? If so then how is it being used? Is the same flex being used on mobile and web? The code will run on every device, it wont crash. But mobile and web will interpret it just slightly differently

  • The "justify-content" value for the parent component, It looks like its currently set to "space-between". CSS properties are inherited from the parent components (maybe a flex value or grid property is being picked up)

  • The height of the FixedFooter component. If it's larger than the page, then the button will be lower

Possible Solutions:

  • Making a separate css for mobile and web, possibly an inline if statement or two separate components with an if statement

  • Change justify-content from "space-between" to either "space-around" or "space-evenly". Check that the value isn't being inherited from parent components and adding it to the specific child component is applicable.

  • Make the hight of the parent component the same height of the page, go off of percentage

Copy link

melvin-bot bot commented Jul 10, 2024

📣 @Daniel-Kline-1! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@Daniel-Kline-1
Copy link

Contributor details
Your Expensify account email: klined49@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0185ba2c752fddc2ff

Copy link

melvin-bot bot commented Jul 10, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@mehedy201
Copy link

mehedy201 commented Jul 10, 2024 via email

@jayeshmangwani
Copy link
Contributor

jayeshmangwani commented Jul 10, 2024

This is a straightforward issue of missing padding at the bottom of the button/parent component. Although all the above proposals will work in this case, I prefer the first working solution. @etCoderDysto 's Proposal of adding a parent view to the button component and add padding bottom of the view looks good to me. We can go with their proposal.

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Jul 10, 2024

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

@cristipaval
Copy link
Contributor

This is a super simple issue. I'll make it $125.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 11, 2024
Copy link

melvin-bot bot commented Jul 11, 2024

📣 @etCoderDysto You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@etCoderDysto
Copy link
Contributor

I will raise a PR ASAP.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jul 12, 2024
@etCoderDysto
Copy link
Contributor

@jayeshmangwani PR is ready for review

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jul 17, 2024
@melvin-bot melvin-bot bot changed the title [$125] Transfer owner - "Continue" button is missing bottom margin [HOLD for payment 2024-07-24] [$125] Transfer owner - "Continue" button is missing bottom margin Jul 17, 2024
Copy link

melvin-bot bot commented Jul 17, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 17, 2024
Copy link

melvin-bot bot commented Jul 17, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.7-8 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 2024-07-24. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jul 17, 2024

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:

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

@jayeshmangwani jayeshmangwani mentioned this issue Jul 17, 2024
50 tasks
@jayeshmangwani
Copy link
Contributor

  • [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR: Add payment card #42771
  • [@jayeshmangwani] 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: Add payment card #42771 (comment)
  • [@jayeshmangwani] 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: N/A
  • [@jayeshmangwani] Determine if we should create a regression test for this bug. No
  • [@jayeshmangwani] 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. N/A

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 23, 2024
@miljakljajic
Copy link
Contributor

Payment summary below

@jayeshmangwani is owed 125 USD for their work reviewing this issue and PR

@miljakljajic
Copy link
Contributor

@etCoderDysto what is your upwork profile? It doesnt seem like you've applied to the job yet? https://www.upwork.com/ab/applicants/1811060685379521358/applicants?nav_dir=pop

@etCoderDysto
Copy link
Contributor

etCoderDysto commented Jul 24, 2024

https://www.upwork.com/jobs/~01851c277cf7d0a07b

I have applied to job now @miljakljajic. And here is link to my Upwork profile.

Copy link

melvin-bot bot commented Jul 24, 2024

Payment Summary

Upwork Job

BugZero Checklist (@miljakljajic)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1811060685379521358/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@jayeshmangwani
Copy link
Contributor

Requested $125

@JmillsExpensify
Copy link

$125 approved for @jayeshmangwani based on this comment.

@miljakljajic
Copy link
Contributor

thank you @etCoderDysto once you accept the offer I will pay it right away!

@etCoderDysto
Copy link
Contributor

I have accepted the offer. Thank you @miljakljajic

@miljakljajic
Copy link
Contributor

everyone paid, no tests required, closing out.

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