-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Comments
I imagine I will be able to start on this next week. |
I forgot to assign you to the issue, @thiagobrez . Sorry for that |
I'll set this one on hold until the second phase is done. |
Starting to investigate this today, while second phase PR is in review |
Good news, these lists are fitting well with the new components. Group members list is ready |
Update: New Chat list is ready. Will be OOO for the rest of the week, so I'll come back to this on Monday |
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) ⬇️ 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! |
No progress there yet, so let's keep the New Group how it is for now. |
Perfect, thanks for the quick response! |
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:
|
Update: Phase 3 is almost finished, will probably already raise the PR once Phase 2 is merged. |
Update: Phase 3 is almost finished, will probably already raise the PR once Phase 2 is merged. |
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! |
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. |
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. |
Should this be closed? If not, what is the next step, who is doing it, and when? |
@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. |
yep, it was reviewed 2 weeks ago but then a new feature was added in the deprecated component. Need to re-review now |
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. |
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. |
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. |
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. |
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! |
Can we close this issue out yet? |
Refactor is complete. I'm fine closing it. |
Payment Summary:C+ Review
Total: $4500 Payment Method: Please request in Newdot Contributor@lukemorawski Agency contributor, no payment owed |
$4,5000 approved for @rushatgabhane |
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:
Thoroughly discussed in the parent issue: #11795 (comment)
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @cristipavalThe text was updated successfully, but these errors were encountered: