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

[Feedback] New autocomplete #773

Closed
17 of 20 tasks
j-f1 opened this issue Mar 17, 2023 · 20 comments
Closed
17 of 20 tasks

[Feedback] New autocomplete #773

j-f1 opened this issue Mar 17, 2023 · 20 comments
Assignees
Labels
help wanted Extra attention is needed tech debt Technical debt we should pay down user interface Related to the user interface

Comments

@j-f1
Copy link
Contributor

j-f1 commented Mar 17, 2023

Introduced in #741.

Remaining bugs/tasks (comment below to add to the list!)

Autocomplete:

PayeeAutocomplete:

GenericInput:

  • When the field is freeform, the suggestion menu shouldn’t appear? (maybe)

Components that use the old autocomplete:

@j-f1 j-f1 added the tech debt Technical debt we should pay down label Mar 17, 2023
@MatissJanis MatissJanis added help wanted Extra attention is needed user interface Related to the user interface and removed needs triage labels Mar 17, 2023
@MatissJanis MatissJanis changed the title Enable new payee autocomplete by default Enable new autocomplete by default Mar 17, 2023
@MatissJanis
Copy link
Member

I expanded the scope of this issue to be the full migration to the new autocomplete component. Not just PayeeAutocomplete.

Hope you don't mind.

@j-f1 j-f1 changed the title Enable new autocomplete by default Enable the new autocomplete by default Mar 18, 2023
@MatissJanis MatissJanis self-assigned this Mar 18, 2023
@MatissJanis
Copy link
Member

not sure the “x” button provides much value and it is super weird (it acts on mouse down + delay rather than mouse up), maybe it can be removed?

I actually view the "x" button more as a feature, not a bug. It's a convenient way to clear the autocomplete selection.

MatissJanis added a commit that referenced this issue Mar 18, 2023
The final `Autocomplete` refactors. After this is merged what's
remaining is to do extensive testing and address the bugs in
#773

This PR moves `Nordigen` autocomplete to the new one without using a
feature flag. IMO this is a safe change given the simple nature of the
Nordigen autocomplete component.
MatissJanis added a commit that referenced this issue Mar 18, 2023
Enabling the new autocomplete for dev/preview deployments.

This will allow us to spot any more issues there might be before we
release the new autocomplete.

#773
@MatissJanis
Copy link
Member

global undo/redo don’t work when focused on a text field

Could you elaborate on this one?

@j-f1
Copy link
Contributor Author

j-f1 commented Mar 19, 2023

Sure!

  1. Delete a transaction
  2. Focus a new autocomplete field
  3. Press cmd+z or ctrl+z

Expected behavior: undoes the deletion. Actual behavior: nothing (compare with handling of old autocomplete)

@albertogasparin
Copy link
Contributor

albertogasparin commented Mar 24, 2023

[RESOLVED]

Have another bug - request for the new autocomplete: allow the dropdown to have a min width, expanding past the input. Reason being when viewing the table on narrow screens (eg iPad) the dropdown is so small that each row wraps into 3, making it hard to scroll/select.

@MatissJanis
Copy link
Member

@albertogasparin good idea: #834

MatissJanis added a commit that referenced this issue Mar 31, 2023
Implement min-width for the autocomplete to make it look better on small
screens.


#773 (comment)

<img width="304" alt="Screenshot 2023-03-31 at 19 02 06"
src="https://user-images.githubusercontent.com/886567/229195868-7d858f18-0c1a-4a9d-95be-5dd0e4aef92c.png">
@MatissJanis
Copy link
Member

Sure!

  1. Delete a transaction
  2. Focus a new autocomplete field
  3. Press cmd+z or ctrl+z

Expected behavior: undoes the deletion. Actual behavior: nothing (compare with handling of old autocomplete)

Ok, looked more into this. It's interesting..

Yes, the old autocomplete allowed global "ctrl+z" operation. But judging by this code - it was actually a bug. Since we explicitly capture undo events that happen in input boxes. I presume it's so the "undo" would apply only to the local input box and would not affect the global state.

With the new autocomplete this bug is fixed. Which means we are no longer able to do global undo operation if the autocomplete field is focused.

Given all this: I think this issue is a wont-fix. WDYT?

@j-f1
Copy link
Contributor Author

j-f1 commented Mar 31, 2023

That’s reasonable.

@MatissJanis
Copy link
Member

@j-f1 I don't recall what was this one. Can you jog my memory?

When the field is freeform, the suggestion menu shouldn’t appear? (maybe)

@j-f1
Copy link
Contributor Author

j-f1 commented Mar 31, 2023

When you filter by Notes → “one of” you get this UI. I think that line was about potentially getting rid of the “Add ‘test’” popup but I’m not sure if it’s a good idea.

Screenshot_2023-03-31 17 07 47

@MatissJanis
Copy link
Member

When scrolling the focused row out of view it (sometimes) suddenly jumps back into view. The old component gracefully drops focus and picks it up when scrolled back into view

Managed to reproduce it, but it also happens in the old autocomplete (see demo below). Basically it has to do with how the table virtualizes and animates the rows outside of the viewport.. When a row is moved outside of the viewport - the row (including autocomplete) is unmounted. And then re-mounted again. Which causes this flash.

IMO this will be addressed once we get to the table migration. As for the autocomplete: I would put this as out-of-scope. WDYT?

Screen.Recording.2023-04-01.at.19.20.31.mov

@j-f1
Copy link
Contributor Author

j-f1 commented Apr 1, 2023

Sure, fine with me!

@j-f1
Copy link
Contributor Author

j-f1 commented Apr 6, 2023

[RESOLVED]

New issue: the autocomplete is a different height than regular inputs. This is mainly noticeable in the rule editor, where the condition/action rows change height when switching between e.g. payee is and date is and notes is

@MatissJanis MatissJanis changed the title Enable the new autocomplete by default [Feedback] New autocomplete Apr 6, 2023
@MatissJanis MatissJanis pinned this issue Apr 6, 2023
MatissJanis added a commit to actualbudget/docs that referenced this issue Apr 6, 2023
**Actual has now been moved to a stand-alone Docker organization. If you
were previously using `jlongster/actual-server` docker image - please
update it to `actualbudget/actual-server`.**

The release has the following notable features:

- Rules can now optionally be applied when any of their conditions match
(in addition to the existing option to apply when all of their
conditions match)
- Rules: quick-create option from the transaction table (in the “X
selected” menu that shows up after selecting a transaction, choose
“Create rule”)
- Ability to hide decimal places for currencies with large numbers (in
Settings → Formatting)
- New autocomplete component (please report any bugs
[here](actualbudget/actual#773))
- Lots of smaller improvements and bugfixes

---------

Co-authored-by: Jed Fox <git@jedfox.com>
@rich-howell
Copy link
Contributor

When adding a transaction to the transaction table, generally I will type my Payee Name E.G. Cambrian Heritage Railways there is sometimes already a Payee with that name in the list but for some reason I am presented with the following dropdown

image

There is only one of the Payee in the list, it is an exact match, that should be selected, not sure why I am presented with the option to create the Payee with a name that distinctly already exists.

I am sure it didn't used to be like this ....

@jnv
Copy link

jnv commented Apr 12, 2023

Hi, since I've updated to the April version of Actual, the new autocomplete in category select causes breaks my keyboard-centered workflow when entering a transaction.

My workflow used to be following:

  1. I start creating a new transaction
  2. Enter payee, hit Tab
  3. Enter Note, hit Tab

At this point Category is usually populated either from payee or note.

  1. Since category is already populated, I just hit Tab again
  2. Enter amount in Payment, Tab, ignore Deposit, Tab, space to check Cleared, Enter to add the transaction

Now with the new Autocomplete component, there's a problem in Step 5: If the category is populated by Note, moving focus to the Category pre-selects Split Transaction, so hitting Tab creates a split transaction. To my knowledge, there's no way to turn a split transaction back to a regular transaction, so I need to cancel the transaction and start a new one.

This issue doesn't happen when the category gets selected by Payee, so I think it has to do with delay in rule evaluation (when the category select gets focused before the category is automatically selected).

I recorded the problem to better illustrate my point:

Screencast.from.2023-04-12.12-41-13.webm

@j-f1
Copy link
Contributor Author

j-f1 commented Apr 12, 2023

To my knowledge, there's no way to turn a split transaction back to a regular transaction, so I need to cancel the transaction and start a new one.

You should be able to hit ctrl/cmd+Z to undo! But I agree this should be fixed.

@jnv
Copy link

jnv commented Apr 12, 2023

You should be able to hit ctrl/cmd+Z to undo! But I agree this should be fixed.

That was my thinking exactly. Undo works for removing an added transaction, but doesn't work for changing an uncommitted split transaction back to a regular one.

@pcchristie
Copy link

I am having @jnv 's issue but also another similar but different one. I used to start at the bottom of my recently imported transactions and shift+tab back through the transactions until I get to the top.

When I am entering a category, let's say "Food" I would previously type "Fo" and food would suggest autocomplete. At that point I should shift+tab (or tab) past it and it should fill. Instead, it removes my input and restores the category to blank. I swear it used to work "properly." Has anyone else noticed this?

It works well with Enter, but not with tab anymore.

@MatissJanis
Copy link
Member

Both of the last two reported issues also relate to react-select not allowing us to natively control the "active"/"hovered" element. I'll see if there's any workaround here..

#773 (comment) & #773 (comment)

@MatissJanis
Copy link
Member

👋 Calling off the new-autocomplete (react-select) experiment as a failure. Reverting back to previous autocomplete and fixing some things in the old one.

We might sometime in the future consider other alternatives. But clearly react-select is not exactly what we want. It has too many limitations that are tricky to work around.

#916

@j-f1 j-f1 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2023
@j-f1 j-f1 unpinned this issue Apr 18, 2023
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this issue Mar 7, 2024
The final `Autocomplete` refactors. After this is merged what's
remaining is to do extensive testing and address the bugs in
actualbudget#773

This PR moves `Nordigen` autocomplete to the new one without using a
feature flag. IMO this is a safe change given the simple nature of the
Nordigen autocomplete component.
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this issue Mar 7, 2024
…dget#789)

Enabling the new autocomplete for dev/preview deployments.

This will allow us to spot any more issues there might be before we
release the new autocomplete.

actualbudget#773
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this issue Mar 7, 2024
Implement min-width for the autocomplete to make it look better on small
screens.


actualbudget#773 (comment)

<img width="304" alt="Screenshot 2023-03-31 at 19 02 06"
src="https://user-images.githubusercontent.com/886567/229195868-7d858f18-0c1a-4a9d-95be-5dd0e4aef92c.png">
my2 added a commit to my2/actual that referenced this issue Aug 13, 2024
The final `Autocomplete` refactors. After this is merged what's
remaining is to do extensive testing and address the bugs in
actualbudget/actual#773

This PR moves `Nordigen` autocomplete to the new one without using a
feature flag. IMO this is a safe change given the simple nature of the
Nordigen autocomplete component.
my2 added a commit to my2/actual that referenced this issue Aug 13, 2024
Enabling the new autocomplete for dev/preview deployments.

This will allow us to spot any more issues there might be before we
release the new autocomplete.

actualbudget/actual#773
my2 added a commit to my2/actual that referenced this issue Aug 13, 2024
Implement min-width for the autocomplete to make it look better on small
screens.


actualbudget/actual#773 (comment)

<img width="304" alt="Screenshot 2023-03-31 at 19 02 06"
src="https://user-images.githubusercontent.com/886567/229195868-7d858f18-0c1a-4a9d-95be-5dd0e4aef92c.png">
bestbooksandcourses added a commit to bestbooksandcourses/actual that referenced this issue Aug 16, 2024
The final `Autocomplete` refactors. After this is merged what's
remaining is to do extensive testing and address the bugs in
actualbudget/actual#773

This PR moves `Nordigen` autocomplete to the new one without using a
feature flag. IMO this is a safe change given the simple nature of the
Nordigen autocomplete component.
bestbooksandcourses added a commit to bestbooksandcourses/actual that referenced this issue Aug 16, 2024
Enabling the new autocomplete for dev/preview deployments.

This will allow us to spot any more issues there might be before we
release the new autocomplete.

actualbudget/actual#773
bestbooksandcourses added a commit to bestbooksandcourses/actual that referenced this issue Aug 16, 2024
Implement min-width for the autocomplete to make it look better on small
screens.


actualbudget/actual#773 (comment)

<img width="304" alt="Screenshot 2023-03-31 at 19 02 06"
src="https://user-images.githubusercontent.com/886567/229195868-7d858f18-0c1a-4a9d-95be-5dd0e4aef92c.png">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed tech debt Technical debt we should pay down user interface Related to the user interface
Projects
None yet
Development

No branches or pull requests

6 participants