matrix-react-sdk
matrix-react-sdk copied to clipboard
matrix-org
Add context menu to breadcrumbs
Signed-off-by: Jaiwanth [email protected]
Here's what your changelog entry will look like:
✨ Features
- Add context menu to breadcrumbs (#6058). Contributed by @jaiwanth-v.
Do you have any accessibility concerns or feedback on the implementation here? Between focus context, how tabbing will behave after right clicking, etc?
I haven't tested the build but wrote the context menu and roving tab index stuff, it should all happily interplay in the correct manner. For keyboard or screen reader users it'd behave identically, except also have a context menu which can be opened using the keyboard shortcut for such or a simulated right click.
The right click interaction is unreliable and causes the UI to glitch (scroll bar can flicker, native menu can flicker, etc). I'd imagine the testing, engineering and maintenance cost of this to be great. Is there anything we can learn from the right click implementation on the space panel which could help?
Indeed the right click after right clicking behaviour looks strange, its not a problem we seem to have on the space panel or room list tiles, so a solution should be found by copying those.
Indeed the right click after right clicking behaviour looks strange, its not a problem we seem to have on the space panel or room list tiles, so a solution should be found by copying those.
Most of this code is adapted from the RoomTile itself so I'm not really sure what went wrong. The scrollbar flicker is unavoidable afaik, and it's also noticeable in the room list on multiple right-clicks.
- Flickering of UI elements.
@nadonomy I'll see if I can do anything about the tooltip label flicker.
oh yes indeed, its sporadic on the room list too, probably quite non-trivial to avoid altogether
@jaiwanth-v thanks again for your contribution. Are you still interested in following up on this?
Sure. At the time, it seemed to me like the issues mentioned were unavoidable. If there is any reasonable solution, please let me know. I'd be happy to implement.
On Thu, Mar 2, 2023 at 11:46 PM Johannes Marbach @.***> wrote:
@jaiwanth-v https://github.com/jaiwanth-v thanks again for your contribution. Are you still interested in following up on this?
— Reply to this email directly, view it on GitHub https://github.com/matrix-org/matrix-react-sdk/pull/6058#issuecomment-1452327609, or unsubscribe https://github.com/notifications/unsubscribe-auth/APU6SOLGFXZEOD2TDBKCDLDW2DPYRANCNFSM45CS32SA . You are receiving this because you were mentioned.Message ID: @.***>
@jaiwanth-v, right, I think these should be fine given that the experience is the same on the room list currently. Could you merge develop into your branch so that we can re-test it?
I'm really sorry, I don't think I'll be able to work on it right now. Someone else can take it up.
On Thu, Mar 9, 2023 at 8:37 PM Johannes Marbach @.***> wrote:
@jaiwanth-v https://github.com/jaiwanth-v are you still planning to work on this?
— Reply to this email directly, view it on GitHub https://github.com/matrix-org/matrix-react-sdk/pull/6058#issuecomment-1462216247, or unsubscribe https://github.com/notifications/unsubscribe-auth/APU6SONPXN3BSEIKTUSPZ3LW3HW2DANCNFSM45CS32SA . You are receiving this because you were mentioned.Message ID: @.***>
I'll be closing this for now to help reduce the number of unfinished PRs. Somebody else may follow up separately.
Thanks for your contribution in any case @jaiwanth-v! 🙏