search

sjdemartini / mui-tiptap

A Material UI (MUI) styled WYSIWYG rich text editor, using Tiptap
MIT License
319 stars 43 forks source link

Add TableCellImproved, TableHeaderImproved extensions and MenuSelectTableBorderStyle control #185

Closed bryanjtc closed 1 week ago

bryanjtc commented 11 months ago

Adds two new extensions and one control to modify the table border style.

Relates to https://github.com/sjdemartini/mui-tiptap/issues/184

bryanjtc commented 11 months ago

The MenuSelect and Extension work as expected.

https://github.com/sjdemartini/mui-tiptap/assets/49354825/fc0761c5-cf5b-434d-9327-e19916d39853

I have a small problem with the table bubble menu, It keeps closing when I open the menu select control. Any idea on how to fix it? @sjdemartini

https://github.com/sjdemartini/mui-tiptap/assets/49354825/ea558c6e-37bf-43a6-a506-475b9ae8cc41

sjdemartini commented 10 months ago

@bryanjtc Nice work getting what you have together so far!

That's a tricky issue. As you've probably seen, the logic for the controlling the table bubble menu open is whether (1) the editor is in focus and (2) Tiptap's state shows a "table" element is active, which is based on the current caret/selection location: https://github.com/sjdemartini/mui-tiptap/blob/876722786cfaa4c248dec3764514224be3241c6f/src/TableBubbleMenu.tsx#L149

In this case, the editor is losing focus due to the Select's Menu (which uses a Popover). You can see this by removing the isEditorFocusedDebounced && part of the condition. I'm not sure what the best option would be in this case. In general it seems we shouldn't keep bubble menus open if the user has clicked away from the editor, so I think that part of the logic is generally useful, though that's obviously causing this undesirable side effect here. Maybe the open={...} logic could be extended somehow to account for a case that there's a menu open? I'm not sure what the right way to do that would be. Do you have any ideas?

sjdemartini commented 10 months ago

Side note: if the table bubble menu is appearing near the bottom of the page currently, and the border style select is opened, it overlaps in an awkward way that can make it tricky to interact with (though maybe partially because of the MenuSelect's tooltip logic):

image

I'm not sure what the right UI/UX should be since this is leading to "nested" menus. 🤔

bryanjtc commented 10 months ago

I refactored the code and created another bubble menu inside the table bubble menu. It works better.

https://github.com/sjdemartini/mui-tiptap/assets/49354825/1355a96f-322b-413a-a27f-66ba13dff3f1

bryanjtc commented 10 months ago

@sjdemartini Can you review the pr? I now that it doesn't have comments, i'm waiting for review to add the comments. Just in case there is something else that needs to be moved or changed.

sjdemartini commented 10 months ago

@bryanjtc Cool, wasn't sure if you'd settled in a place where you thought it was ready for review again. Things have been super busy on my end unfortunately, but I'll try to review some time soon (hopefully next week). Sorry for the delay.

bryanjtc commented 10 months ago

Nice that you took the time to review everything. I will try to push the fixes/changes today and reply/resolve each conversatioon.

sjdemartini commented 1 week ago

Since there hasn't been activity in a while, I'm going to close this out. Thanks again for looking into it, since it can be a useful model if folks want to add their own implementation of these bubble menus to use elsewhere.