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-03-20] [LOW] Enable pagination on SelectionList #36121

Closed
cristipaval opened this issue Feb 8, 2024 · 24 comments
Closed
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering Weekly KSv2

Comments

@cristipaval
Copy link
Contributor

cristipaval commented Feb 8, 2024

Problem

As part of this issue, we refactored the old gigantic OptionItem component into multiple components focused on specific use cases. The refactoring has taken quite a long time, and in the meantime, pagination was added to the App to make the experience smoother on high-traffic accounts.
Currently, the new component SelectionList doesn't have pagination, and the Search feature in high-traffic accounts doesn't offer the best experience.

Solution

Let's enable pagination on the new SelectionList component.

cc @mountiny

Issue OwnerCurrent Issue Owner: @cristipaval
@cristipaval cristipaval added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 8, 2024
@cristipaval cristipaval self-assigned this Feb 8, 2024
Copy link

melvin-bot bot commented Feb 8, 2024

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

@lukemorawski
Copy link
Contributor

@cristipaval reporting for duty 🖖

@cristipaval
Copy link
Contributor Author

Do we have an update here, @lukemorawski ?

@melvin-bot melvin-bot bot removed the Overdue label Feb 12, 2024
@lukemorawski
Copy link
Contributor

@cristipaval no, sorry, had a more urgent bug fixes. I will be tackling that today

@melvin-bot melvin-bot bot added the Overdue label Feb 15, 2024
Copy link

melvin-bot bot commented Feb 16, 2024

@abekkala, @cristipaval, @lukemorawski Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@cristipaval
Copy link
Contributor Author

I know Callstack devs are quite swamped atm, so most probably, @lukemorawski will have an update on Monday.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Feb 16, 2024
@lukemorawski
Copy link
Contributor

@cristipaval yep, I was ooo on Friday. There are also couple of regressions I need address quickly. This ticket is next up in my queue.

@melvin-bot melvin-bot bot removed the Overdue label Feb 19, 2024
@greg-schroeder greg-schroeder changed the title Enable pagination on SelectionList [LOW] Enable pagination on SelectionList Feb 20, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 21, 2024
@abekkala
Copy link
Contributor

@lukemorawski can you provide an update?

@melvin-bot melvin-bot bot removed the Overdue label Feb 21, 2024
@lukemorawski
Copy link
Contributor

@abekkala started woking on it, but I need to resolve some complicated conflicts on another PR. I should have a PR today in the evening or tomorrow

Copy link

melvin-bot bot commented Feb 22, 2024

@abekkala @cristipaval @lukemorawski this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@abekkala
Copy link
Contributor

thanks @lukemorawski

@melvin-bot melvin-bot bot added the Overdue label Feb 26, 2024
@cristipaval
Copy link
Contributor Author

👀

@melvin-bot melvin-bot bot added the Overdue label Feb 28, 2024
@abekkala
Copy link
Contributor

@lukemorawski is there any update on this one?

@melvin-bot melvin-bot bot removed the Overdue label Feb 28, 2024
Copy link

melvin-bot bot commented Feb 29, 2024

@abekkala @cristipaval @lukemorawski this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

@abekkala
Copy link
Contributor

@lukemorawski do you still have some higher priority PRs that is putting this one off for a bit?

@lukemorawski
Copy link
Contributor

Hey. Yes, but I posting one PR today and I'll be waiting for a review, so I should be able to finish this. Fingers crossed 🤞

@lukemorawski
Copy link
Contributor

@abekkala PR is open

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Overdue labels Mar 1, 2024
@abekkala
Copy link
Contributor

abekkala commented Mar 1, 2024

Great @lukemorawski ! Thanks!

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Mar 13, 2024
@melvin-bot melvin-bot bot changed the title [LOW] Enable pagination on SelectionList [HOLD for payment 2024-03-20] [LOW] Enable pagination on SelectionList Mar 13, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Mar 13, 2024
Copy link

melvin-bot bot commented Mar 13, 2024

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

Copy link

melvin-bot bot commented Mar 13, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.51-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 2024-03-20. 🎊

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

Copy link

melvin-bot bot commented Mar 13, 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:

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

@abekkala
Copy link
Contributor

PAYMENT SUMMARY FOR MAR 20

@lukemorawski can you complete the checklist above?

@abekkala
Copy link
Contributor

@fedirjh payment sent and contract ended - thank you! 🎉

@abekkala
Copy link
Contributor

no regression test needed

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. Engineering Weekly KSv2
Projects
No open projects
Development

No branches or pull requests

3 participants