ternandsparrow / wild-orchid-watch-pwa

Progressive Web App for the Wild Orchid Watch citizen science project
https://app.wildorchidwatch.org
4 stars 4 forks source link

Context menu no longer opens #99

Closed tomsaleeba closed 2 years ago

tomsaleeba commented 2 years ago

Env Commit eb206c80369cc191e2f9c00f130ca7792600136e Chrome 101 on Linux

Steps

  1. launch app and login
  2. open the details page for an observation (tap on it)
  3. click the 3 vertical dots icon in the top, right of the page to open the context menu image

Expected: The menu opens at the bottom of the screen, no errors in the browser console. Here's a screen capture from beta (https://beta.app.wildorchidwatch.org/) which is not broken. menu-working

Actual: The menu does not open and this error appears in the console

Uncaught (in promise) TypeError: Animator is not a constructor
    at AnimatorFactory.newAnimator (animator-factory.js?a1bf:104:1)
    at eval (base-dialog.js?c252:125:1)
    at DoorLock.waitUnlock (doorlock.js?e9de:86:1)
    at eval (base-dialog.js?c252:123:1)
    at new Promise (<anonymous>)
    at HTMLElement._setVisible (base-dialog.js?c252:122:1)
    at HTMLElement.show (base-dialog.js?c252:83:1)
    at eval (action-sheet.js?59eb:96:1)
    at eval (setImmediate.js?41ca:48:1)
    at runIfPresent (setImmediate.js?41ca:67:1)

I recently updated Onsen, the UI library that gives us this menu, so my guess is that broke something.

CC @tokmakoff

sbbu commented 2 years ago

The bug seems to be in Onsen breaking all action sheets, I wasn't able to figure it out.. I checked the documentation and it doesn't look like the syntax for action sheets has changed at all (https://onsen.io/v2/api/js/ons-action-sheet.html). I also tried reverting versions for onsenui and vue-onsenui, but it didn't help.

tomsaleeba commented 2 years ago

The offending line in the source (https://github.com/OnsenUI/OnsenUI/blame/master/onsenui/esm/ons/internal/animator-factory.js#L104) hasn't been touched in 7 years. The whole file has been moved a few times recently but hasn't been meaningfully changed in a long time :shrug:.

The next thing to trace would be the this._animators param. But we've got more important things.

Brainstorming a workaround, we could stop using the context menu and just put buttons for the actions either in the toolbar or on the page. Then we don't care if the context menu is broken.

tomsaleeba commented 2 years ago

Comment from meeting: let's remove the menu and swap to buttons.

sbbu commented 2 years ago

I've removed the context menu and made delete its own button, which now works: 906fe2285deae062df24013271fcfca36ff56296. Delete was the only option in the menu, so it makes more sense to just have it as its own button anyway.