Page MenuHomePhabricator

[Components] Watchlist: Remove unneeded `WKMenuButtonDelegate` method
Closed, DeclinedPublic


We should avoid needing to have an extra wkSwiftUIMenuButtonUserDidTapAccessibility delegate method just for accessibility purposes. This adds additional requirements to any view that wants to use a menu button. Instead, we should figure out how to refactor and improve the menu button to transparently support the needed behavior.

I believe we can achieve this by refactoring the WKMenuButton itself to allow its MenuItems to have arbitrary metadata, similar to how the WKMenuButton itself does. This also removes the need to do "title" checking to figure out what action they should perform. See more notes here:

Once this work is removed in WKMenuButton, we should also remove its use in the Watchlist feature itself.

Event Timeline

Dmantena renamed this task from Watchlist/Components: Remove unneeded `WKMenuButtonDelegate` method to [Components] Watchlist: Remove unneeded `WKMenuButtonDelegate` method.Nov 3 2023, 7:11 PM

Let's hold off on this refactor until after our Components button work is underway. The decisions from that work will naturally inform the implementation here. Adding as child task to Components Library epic and marking as resolved from outstanding issues for now as it does not change functionality.

Going to close this one, as we have bigger fish to fry with technical debt and I don't expect us to pick this up anytime soon. In the meantime, delegates of the button can provide an empty implementation if they don't need to react to the accessibility tap. We should eventually consider a consistent, (ideally) built-in approach to handling all impressions and button taps for logging purposes.