MenuItem use of a vs. button

[edoardo]

While refactoring DV to use the new FileMenu based on ui, I started getting some failures in DV's cypress tests.
One test in particular is testing the Esc keystroke for closing the menu (functionality currently implemented in the FileMenu).

The test failed because cypress cannot type on an anchor element without an href attribute.
This made me wonder why MenuItem is using the anchor element, I see href is listed as prop for MenuItem, but in all the cases I'm dealing with of menus in apps, the menu item only needs to trigger a callback on click, it never needs to send the user somewhere.
Using a in the typical menu use case does not seem semantically correct.

Should MenuItem use button instead when no href prop is passed?

[martinkrulltott]

An a should only be used with an href (unless it's only acting as a placeholder anchor). Thus, if no href is provided and instead an onClick is used it's no longer a valid a. In this case a button should indeed be used.
I think the suggestion to switch the element type used based on if href is passed or not is a good idea 👍

[varl]

Should MenuItem use button instead when no href prop is passed?

I agree, and would approve a PR with that change. 👍

[Mohammer5]

The component needs to be updated -> A button should be used when href is empty / not provided.
Issue for this task: https://jira.dhis2.org/browse/LIBS-117