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

LOW: [$500] Selection List Refactor: Simple Selection List #20354

Closed
thiagobrez opened this issue Jun 7, 2023 · 130 comments
Closed

LOW: [$500] Selection List Refactor: Simple Selection List #20354

thiagobrez opened this issue Jun 7, 2023 · 130 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Monthly KSv2 Reviewing Has a PR in review

Comments

@thiagobrez
Copy link
Contributor

thiagobrez commented Jun 7, 2023

This issue keeps track of Phase 3 of the Selection List Refactor, in which we are refactoring all different list component variations into 3 new, clean components:

  • Phase 1: Radio Button List
  • Phase 2: Checkbox List
  • Phase 3: Simple Selection List (this phase)

Thoroughly discussed in the parent issue: #11795 (comment)

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a737edd58e0c740d
  • Upwork Job ID: 1698702807486967808
  • Last Price Increase: 2023-09-04
Issue OwnerCurrent Issue Owner: @cristipaval
@thiagobrez
Copy link
Contributor Author

thiagobrez commented Jun 30, 2023

I imagine I will be able to start on this next week.

@cristipaval
Copy link
Contributor

I forgot to assign you to the issue, @thiagobrez . Sorry for that

@melvin-bot melvin-bot bot removed the Overdue label Jun 30, 2023
@melvin-bot melvin-bot bot added the Overdue label Jul 10, 2023
@cristipaval
Copy link
Contributor

I'll set this one on hold until the second phase is done.

@melvin-bot melvin-bot bot removed the Overdue label Jul 11, 2023
@cristipaval cristipaval changed the title Selection List Refactor: Simple Selection List [HOLD App issue #20353] Selection List Refactor: Simple Selection List Jul 11, 2023
@thiagobrez
Copy link
Contributor Author

Starting to investigate this today, while second phase PR is in review

@thiagobrez
Copy link
Contributor Author

Good news, these lists are fitting well with the new components. Group members list is ready

@thiagobrez
Copy link
Contributor Author

Update: New Chat list is ready. Will be OOO for the rest of the week, so I'll come back to this on Monday

@thiagobrez
Copy link
Contributor Author

thiagobrez commented Jul 25, 2023

Hi @shawnborton!

Context:

This issue is Phase 3 of the lists refactor (simple lists, no selection). One of these lists is the New Chat list (image below) ⬇️

Screenshot 2023-07-25 at 14 19 53

Question:

This list is being reused for both New Chat and New Group lists. So I was wondering if maybe you have an update if I can proceed with the New Group list? Last time it was blocked (here) because you still needed to investigate the patterns around checkbox lists.

If there is not a decision on the New Group list yet, no problem, I can make it just for the New Chat list. I'm just checking because if I could change both at the same time, it would make it simpler :)

Thanks!

@shawnborton
Copy link
Contributor

No progress there yet, so let's keep the New Group how it is for now.

@thiagobrez
Copy link
Contributor Author

Perfect, thanks for the quick response!

@thiagobrez
Copy link
Contributor Author

Update: Phase 3 is almost finished, will probably already raise the PR once Phase 2 is merged.

All previously investigated lists by Arek #11795 (comment) were already replaced (with one exception: SearchPage), but I see some lists were missed from the initial investigation (regarding all phases), so I will be adding those here as well.

  1. SearchPage is currently being migrated to functional component, so I'm waiting this PR to be merged first before replacing the list

  2. Lists in these pages were missed from the initial investigation, so I will be replacing them in this Phase:

  • NotificationsPreferencePage
  • WriteCapabilityPage
  • TaskAssigneeSelectorModal
  • TaskShareDestinationSelectorModal
  • MoneyRequestConfirmationList
  • IOUCurrencySelection
  • MoneyRequestParticipantsSplitSelector

@thiagobrez
Copy link
Contributor Author

Update: Phase 3 is almost finished, will probably already raise the PR once Phase 2 is merged.

@melvin-bot melvin-bot bot added the Overdue label Aug 10, 2023
@thiagobrez
Copy link
Contributor Author

Update: Phase 3 is almost finished, will probably already raise the PR once Phase 2 is merged.

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Apr 11, 2024
Copy link

melvin-bot bot commented Apr 11, 2024

This issue has not been updated in over 15 days. @JmillsExpensify, @cristipaval, @rushatgabhane, @lukemorawski eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

Copy link

melvin-bot bot commented Apr 18, 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 19, 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.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Apr 23, 2024
@quinthar quinthar changed the title [$500] Selection List Refactor: Simple Selection List LOW: [$500] Selection List Refactor: Simple Selection List May 12, 2024
@quinthar
Copy link
Contributor

Should this be closed? If not, what is the next step, who is doing it, and when?

@MrMuzyk
Copy link
Contributor

MrMuzyk commented May 13, 2024

@quinthar Hey, so we have a PR that is waiting to be reviewed and tested again. After this is merged then whole refactor is complete and this issue can be closed.

Note: PR has been opened for 3 weeks now but it was hanging due to merge freeze I guess. In the meantime a new feature has been merged (uneven splits) that did some major changes in the refactored component and added a new feature to deprecated component that was being removed in this PR. I had to start pretty much from scratch thus the request to review and test it again before merging.

@rushatgabhane
Copy link
Member

yep, it was reviewed 2 weeks ago but then a new feature was added in the deprecated component. Need to re-review now

Copy link

melvin-bot bot commented May 15, 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 May 15, 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 May 15, 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 May 16, 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.

@melvin-bot melvin-bot bot removed the Weekly KSv2 label May 16, 2024
Copy link

melvin-bot bot commented May 16, 2024

This issue has not been updated in over 15 days. @JmillsExpensify, @cristipaval, @rushatgabhane, @lukemorawski eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@melvin-bot melvin-bot bot added the Monthly KSv2 label May 16, 2024
@JmillsExpensify
Copy link

Can we close this issue out yet?

@MrMuzyk
Copy link
Contributor

MrMuzyk commented May 20, 2024

Refactor is complete. I'm fine closing it.

@laurenreidexpensify
Copy link
Contributor

Payment Summary:

C+ Review

@rushatgabhane

Total: $4500

Payment Method: Please request in Newdot

Contributor

@lukemorawski Agency contributor, no payment owed

@JmillsExpensify
Copy link

$4,5000 approved for @rushatgabhane

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 Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Monthly KSv2 Reviewing Has a PR in review
Projects
Development

No branches or pull requests