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

etrokal commented 1 month ago

How to reproduce:

Start hledger web
Add a transaction
On the form you should fill the first 4 account fields and amounts
The form should now show a 5th account field and amount
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 commented 1 month ago

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 commented 2 weeks ago

Thanks for the report!

simonmichael commented 1 week ago

@etrokal and for the analysis.

I have implemented a fix that does not work, and I can't see why. Pushed as #2237, I'm open to any ideas..

simonmichael commented 1 week ago

Some hledger-web dev workflow notes for the record:

I used just ghci-web to load hledger-web into GHCI and :main --port 5001 -f examples/sample.journal to start it.
--port 5001 because the default 5000 clashes with airtunes on mac
--port, not -p which means something else
generally CTRL-c, :reload, and :main ... again shows the latest changes
but sometimes my browser (safari) would not update cached files; then I had to use a private window to work around
even private safari windows may not update; need to close and open a new one after each code change
safari inspector can get partially stuck after debugging; need to open a new window
see also https://hledger.org/CODE.html#hledger-web

simonmichael commented 1 week ago

And here's one comment suggesting just calling .typeahead() on the new element should be enough.

simonmichael commented 5 days ago

Fixed in master by the latest #2237.

simonmichael / hledger