-
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
[HOLD for payment 2024-06-28] MEDIUM: Make auto-complete component work inline #16078
Comments
I'll take a look |
This issue has not been updated in over 15 days. 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! |
@perunt how's it going with this one? |
Two related PRs have been opened for this:
After these are merged, it would be possible to develop a general component that can be used for inline suggestions such as emojis or mentions. |
@perunt for the react native PR, is the plan to also try to get that merged upstream? |
I'm not certain how the review process will unfold since it's usually a lengthy one, but I've submitted an upstream pull request |
@perunt would you mind laying out what you see as the path to inline auto-completion? I see we have these two PRs, the first of which is merged, and the second of which is close to being able to merge. What's the plan after that? |
@puneetlath Yes, the first PR has been merged, and the team at Callstack is currently updating the React Native version. These changes should be available in the app in the coming days and will bring those changes into the app. |
Ok cool! |
To improve tracking of this task, I'll provide a live link to the PR that I'm waiting for link |
Any update here @perunt ? Are you able to prioritize this? Looks like we have a handful of issues held on this. Thx |
@mallenexpensify, the PR is in the review process. I don't know how long it could take. If you want, I can start fixing those issues with my changes right now. |
@puneetlath would know better than I. Thx |
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. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.0-9 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-06-28. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
I was C+ reviewer on the PR. @puneetlath Can you assign me to handle payment here |
The PR introduce a regression here #44100. But based on the scale of changes we were doomed to miss small regression like this. |
Assigned! Are there any regression tests you think we should add for this? |
Payment Summary
BugZero Checklist (@puneetlath)
|
It looks like @alitoshmatov ended up doing the review here. So I think @Ollyws isn't due anything, but let me know if I'm mistaken. |
@puneetlath Yes this is new feature, we require a regression tests. Testing steps in PR looks good: Testing Emoji Suggestions:
Testing Mention Suggestions:
|
Awesome, thanks. GH issue for regression test is here: https://github.com/Expensify/Expensify/issues/409359 Upwork offer is here: https://www.upwork.com/nx/wm/offer/102945166 Please ping me on this issue when you've accepted. |
@puneetlath accepted the offer |
Paid. Thanks everyone! |
Our existing auto-complete component works well, but because it has to live above the composer, it limits the size the composer can grow to when expanded. Ideally, the emoji component would work in-line, similar to Slack's so that the composer has the ability to grow to the full height.
cc @perunt
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @puneetlathThe text was updated successfully, but these errors were encountered: