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

[Pay meow][$250] Workspace - Workspace type name is not showing in correct column #40524

Closed
2 of 6 tasks
lanitochka17 opened this issue Apr 19, 2024 · 31 comments
Closed
2 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 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Apr 19, 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: 1.4.63-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): shussain+test@applausemail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

Issue found when executing PR #39991

Action Performed:

  1. Open a workspace join link that we aren't a member of this (workspace from gmail account)
  2. Check the workspace type name

Expected Result:

Workspace type name should not displayed with "requested" lable

Actual Result:

Workspace type name is not showing in correct column

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

Add any screenshot/video evidence

Bug6454111_1713465637610.2024-04-18_23-30-53.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01740751b07e243f13
  • Upwork Job ID: 1781233221316165632
  • Last Price Increase: 2024-04-19
  • Automatic offers:
    • cubuspl42 | Reviewer | 0
Issue OwnerCurrent Issue Owner: @mallenexpensify
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Apr 19, 2024
Copy link

melvin-bot bot commented Apr 19, 2024

Triggered auto assignment to @marcaaron (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@lanitochka17
Copy link
Author

@marcaaron FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@lanitochka17
Copy link
Author

We think that this bug might be related to #wave-collect - Release 1

@marcaaron
Copy link
Contributor

Looks kinda bad so probably should be a blocker?

2024-04-18_15-51-53

Got assigned this towards the end of my day and have not investigated yet.

@luacmartins is this related to Simplified Collect?

@mountiny mountiny assigned mountiny and unassigned marcaaron Apr 19, 2024
@mountiny mountiny added the External Added to denote the issue can be worked on by a contributor label Apr 19, 2024
@melvin-bot melvin-bot bot changed the title Workspace - Workspace type name is not showing in correct column [$250] Workspace - Workspace type name is not showing in correct column Apr 19, 2024
Copy link

melvin-bot bot commented Apr 19, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01740751b07e243f13

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

melvin-bot bot commented Apr 19, 2024

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

@dragnoir
Copy link
Contributor

dragnoir commented Apr 19, 2024

Proposal

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

Workspace type name is not showing in correct column

What is the root cause of that problem?

The styles is limited to the condition !isJoinRequestPending

<View style={[styles.flexRow, isWide && !isJoinRequestPending && styles.flex1, styles.gap2, isNarrow && styles.mr5, styles.alignItemsCenter]}>

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

We need to remove the !isJoinRequestPending and always allow the style

Result:
image

To align exactly the header we should change the padding from 40 to the width of the new Badge 84px [here]. The new 4th column now will fit the Badge. To make sure the 3 dots also fit well, we need to add extra padding/margin to it to be the same width as the new 4th header column and the badge. (

<View style={[styles.flexRow, styles.gap5, styles.p5, styles.pl10, styles.appBG]}>
)

Also we should consider small screen view

image

We need to adjust the Request badge to the top on non-wide screen.

Final Results:

image

image

@mountiny
Copy link
Contributor

I can take over now to help push it forwards

The columns should be aligned, I can see that even in the original PR, there is a misalignment on the workspace type column
image

@mountiny
Copy link
Contributor

@shahinyan11 will your proposal ensure the columns are aligned?

@dragnoir
Copy link
Contributor

@mountiny pls check my updates proposal #40524 (comment)

We need to adjust the table header and we also needs to consider the small size screens lyout.

@mountiny
Copy link
Contributor

@dragnoir Nice! Can you also check how it looks like with a mix of requested and workspaces you are already member of?

@shahinyan11
Copy link
Contributor

@shahinyan11 will your proposal ensure the columns are aligned?

No It doesn't look perfect yet, I'm still working on it

@dragnoir
Copy link
Contributor

@mountiny this is how it looks

image

I will change this area on the header to fit both the Requested BAdge and the 3 dots menu
image

@tienifr
Copy link
Contributor

tienifr commented Apr 19, 2024

Proposal

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

Workspace type name is not showing in correct column

What is the root cause of that problem?

We only add styles.flex1 if isJoinRequestPending is false.

<View style={[styles.flexRow, isWide && !isJoinRequestPending && styles.flex1, styles.gap2, isNarrow && styles.mr5, styles.alignItemsCenter]}>

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

  1. We should remove !isJoinRequestPending here.

<View style={[styles.flexRow, isWide && !isJoinRequestPending && styles.flex1, styles.gap2, isNarrow && styles.mr5, styles.alignItemsCenter]}>

  1. To ensure the column aligns, we should ensure the header column and the item column align. To do that we should
  • Change this style of action column to minWidth: 120

<View style={[styles.ml10, styles.mr2]} />

  • And then here we also add this minWidth style to the wrap of action

<View style={[isNarrow && styles.mr5]}>

  • Add an extra prop in ThreeDotsMenu like containerStyles and add it here

What alternative solutions did you explore? (Optional)

NA

Result

image

@tienifr
Copy link
Contributor

tienifr commented Apr 19, 2024

@mountiny I just updated the result section in proposal. I tested and It works in all cases

@dragnoir Nice! Can you also check how it looks like with a mix of requested and workspaces you are already member of?

@mountiny FYI, with the other proposals (at the time I posted my proposal), it will not align when having this mix.

@dragnoir
Copy link
Contributor

dragnoir commented Apr 19, 2024

@tienifr you copying from my proposal man. The value 120 I am the one I wrote and then I changed it to 84 wish is the correct value. So be sure what to copy :)

Also, I am the first who consider mobile/small screen view.

finally, you didn't mention how to adjust the 4th column which needs adjustments of the width of the 3 dots columns.

@cubuspl42
Copy link
Contributor

I think that the proposal by @dragnoir is good enough. I understand that the @tienifr built on top of it with some improvements, but I don't think they're significant enough to assign them.

So I'd recommend assigning @dragnoir and focusing on testing all corner cases during the PR testing

C+ reviewed 🎀 👀 🎀

Copy link

melvin-bot bot commented Apr 19, 2024

Current assignee @mountiny is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

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

melvin-bot bot commented Apr 19, 2024

📣 @cubuspl42 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Apr 19, 2024

📣 @dragnoir 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 📖

@mountiny
Copy link
Contributor

Thanks for the review!

@dragnoir can you please raise the PR promptly?

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Apr 19, 2024
@mountiny mountiny added the Bug Something is broken. Auto assigns a BugZero manager. label Apr 21, 2024
Copy link

melvin-bot bot commented Apr 21, 2024

Triggered auto assignment to @mallenexpensify (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.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 21, 2024
@mountiny mountiny removed the DeployBlockerCash This issue or pull request should block deployment label Apr 22, 2024
@mountiny
Copy link
Contributor

$250 to @dragnoir and @allroundexperts once the time comes up.

@dragnoir since this is quite simple PR, could you please also address the follow up from @shawnborton mentioned on the PR to update the style of the three dots padding?

Copy link

melvin-bot bot commented Apr 23, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

Copy link

melvin-bot bot commented Apr 30, 2024

@dragnoir, @mallenexpensify, @allroundexperts, @mountiny Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented May 2, 2024

@dragnoir, @mallenexpensify, @allroundexperts, @mountiny Still overdue 6 days?! Let's take care of this!

@mallenexpensify mallenexpensify added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels May 3, 2024
@mallenexpensify mallenexpensify changed the title [$250] Workspace - Workspace type name is not showing in correct column [Pay meow][$250] Workspace - Workspace type name is not showing in correct column May 3, 2024
@mallenexpensify
Copy link
Contributor

Automation failed, payment's due.
@dragnoir , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01740751b07e243f13

@melvin-bot melvin-bot bot removed the Overdue label May 3, 2024
@dragnoir
Copy link
Contributor

dragnoir commented May 3, 2024

@mallenexpensify offer accepted, thank you

@mallenexpensify
Copy link
Contributor

Contributor: @dragnoir paid $250 via Upwork
Contributor+: @allroundexperts owed $250 via NewDot

@JmillsExpensify
Copy link

$250 approved for @allroundexperts

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 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

10 participants