machawk1 / wail

:whale2: Web Archiving Integration Layer: One-Click User Instigated Preservation
https://matkelly.com/wail
MIT License
345 stars 32 forks source link

UI/UX issues with MemGator integration #480

Closed machawk1 closed 3 years ago

machawk1 commented 3 years ago

WAIL's interaction with the included Memento Aggregator (MemGator) is sub-optimal and could further leverage the capability of the tool.

One issue I am noticing requires a primer on the current dynamics. When a user changes the URL in the text field within the Basic interface tab, a call is made to the memgator executable with the URL as a parameter. This causes the interaction of the UI to subtilely lock up, despite the usage of timers and background execution (which ought to be verified).

There are a few other TODOs that would improve the integration, e.g.,:

Another nitpick is that WAIL uses MemGator in "one-off" mode, calling the executable each time instead of running the persistent server process. What the latter might facilitate should be enumerated. Perhaps @ibnesayeed can provide some advantages to running MemGator in server mode in the context of WAIL.

Despite all of this, this issue is to highlight the delay and UI lockup that occurs while waiting for MemGator to return results. Take a look at the threading implementation or whatever method is used.

EDIT: For more info in replicating the "UI locking up", type a letter into the URL then try a second one and note the delay. This was not always an issue, so some regression might be needed to identify when this UX issue was introduced.

machawk1 commented 3 years ago

Oddly, I pulled a fresh copy from this repo's main branch and cannot reproduce the behavior. An updated task: the Status Bar disappears initially after returning results. ezgif-5-be475f133fdd

machawk1 commented 3 years ago

An update on the above. When context is removed from the window, the status bar re-appears. ezgif com-gif-maker

This makes me think there is an issue that is preventing the window from being re-painted when active after the change to the status bar. When switching to another application, the paint action appears to be invoked. wxPython 4.1.0, Python 3.9.0, macOS 10.15.6.

machawk1 commented 3 years ago

ezgif com-gif-maker (1)

Even staying within the app, as soon as the window with the statusbar loses focus, the status bar UI is displayed as expected.

machawk1 commented 3 years ago
def uri_changed(self, event):
...
  self.memgator_delay_timer = threading.Timer(
        1.0, thread.start_new_thread, [self.fetch_mementos, ()]
        )
        self.memgator_delay_timer.daemon = True
        self.memgator_delay_timer.start()

is the source of it clearing but self.fetch_mementos() alone cannot be invoked here, as it is what causes the UI to lockup. The keydown/up/press event should be smarter instead of waiting like this.

machawk1 commented 3 years ago

This could be attributed to the UI being updated from a thread that is not the main thread, which should not be done in Python. Doing the calculation/retrieval, returning the result, then updating the UI using wx.CallAfter might be a better approach.

ibnesayeed commented 3 years ago

Python has a global interpreter lock (GIL), so running a process in a thread is not necessarily going detach your rendition loop. I would suggest you try running MemGator as a background process and communicate data to render in the UI using queue.

Using MemGator in server mode will eliminate the need to run a system process each time which has its bootup and teardown overhead which accumulates if it happens often.

machawk1 commented 3 years ago

@ibnesayeed That will require a bit of overhaul in the logic but one goal is to move to this style of communicating with a persistent MemGator process.

There is currently not a noticeable overhead in executing MemGator like this. The task is to execute MemGator in an async process after a set amount of time (cancelable), return the result, and update the UI from the main thread.

machawk1 commented 3 years ago

In d655d22c5e2b009bd228c96adc72bcd0658bfe5b I changed the call to update the status bar to use wx.CallAfter, which invokes after previous events have completed instead of trying to update the UI from a non-GUI thread. The "non-GUI thread" aspect comes about because the call is invoked using a timer after a short delay, to prevent a call from each interaction of the Basic interface URI text box.

ezgif com-gif-maker (2)