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

UI bug in hledger web - autocomplete not working on the 5th account field and beyond #2215

Open
etrokal opened this issue Aug 8, 2024 · 2 comments
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. impact3 Affects just a few users. severity2 Minor to moderate usability/doc bug, reasonably easy to avoid or tolerate. web The hledger-web tool.

Comments

@etrokal
Copy link

etrokal commented Aug 8, 2024

How to reproduce:

  1. Start hledger web
  2. Add a transaction
  3. On the form you should fill the first 4 account fields and amounts
  4. The form should now show a 5th account field and amount
  5. The 5th account field does not have autocompletion working

Ledger version

hledger 1.34-g7a83578ec-20240601, windows-x86_64

How severe is this bug ?

severity2

Who is likely to be impacted by this bug ?

Since it appeared to fly under the radar for sometime, I believe only a small portion of the user base would be impacted. Most people would be satisfied with the first 4 account fields. So impact3.

Proposed solution

I'm not a Haskell developer, nor do I know which web framework is being used, but the issue seems to be on the javascript side. When adding the new field, we should be destroying the typeahead instance and reloading it to include the new field. Since we are not doing this, the current typeahead instance doesn't know about the new field.

Looking at the source code, I reckon these 2 files should be affected:

  • add-form.hamlet: This is where typeahead is instantiated. The instantiation code should probably be put inside a global function for it to be called from the other file, when adding a new field.
  • hledger.js: I believe this is where we are adding a new field to the form. More specifically on the addformAddPosting function. Here we should be re-instantiating typeahead.

I could do a PR, but I really wouldn't know how to start the server for testing.

@etrokal
Copy link
Author

etrokal commented Aug 13, 2024

Adding to my proposed solution, we should probably reload the typeahead instance before removing fields as well. I don't know the details about typeahead internals, but we could be introducing a memory leak if we remove the field first.

@simonmichael
Copy link
Owner

Thanks for the report!

@simonmichael simonmichael added A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. web The hledger-web tool. severity2 Minor to moderate usability/doc bug, reasonably easy to avoid or tolerate. impact3 Affects just a few users. labels Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. impact3 Affects just a few users. severity2 Minor to moderate usability/doc bug, reasonably easy to avoid or tolerate. web The hledger-web tool.
Projects
None yet
Development

No branches or pull requests

2 participants