hledger icon indicating copy to clipboard operation
hledger copied to clipboard

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

Open etrokal opened this issue 1 year ago • 1 comments
trafficstars

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 avatar Aug 08 '24 18:08 etrokal

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.

etrokal avatar Aug 13 '24 13:08 etrokal

Thanks for the report!

simonmichael avatar Sep 13 '24 22:09 simonmichael

@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 avatar Sep 22 '24 04:09 simonmichael

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 avatar Sep 22 '24 04:09 simonmichael

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

simonmichael avatar Sep 22 '24 04:09 simonmichael

Fixed in master by the latest #2237.

simonmichael avatar Sep 24 '24 19:09 simonmichael