search

outline / rich-markdown-editor

The open source React and Prosemirror based markdown editor that powers Outline. Want to try it out? Create an account:
https://www.getoutline.com
BSD 3-Clause "New" or "Revised" License
2.87k stars 588 forks source link

fix: table selection support format options #491

Closed JiangWeixian closed 3 years ago

JiangWeixian commented 3 years ago

https://github.com/outline/rich-markdown-editor/issues/490

tommoor commented 3 years ago

Nice, thanks! I think we need to add a separator item between the table controls and the formatting controls to complete this

JiangWeixian commented 3 years ago

https://github.com/outline/rich-markdown-editor/blob/9dd889932ebc688aaa1082108b24258e9b85aa1b/src/components/SelectionToolbar.tsx#L161-L163

@tommoor however, according to the above code, separator item will be filtered out.

tommoor commented 3 years ago

Looks like that bug was recently introduced with the code for disabling extensions 🙈

tommoor commented 3 years ago

image

Really appreciate the work here, and for finding the separator bug. Unfortunately after testing it out locally I just don't think this is going to work from a UX point of view. The toolbar is totally overwhelming and creates a bunch of extra edge cases – (eg link button doesn't work) – I'm confident it's better to just select the text directly and keep the toolbars separate.

JiangWeixian commented 3 years ago

image

Really appreciate the work here, and for finding the separator bug. Unfortunately after testing it out locally I just don't think this is going to work from a UX point of view. The toolbar is totally overwhelming and creates a bunch of extra edge cases – (eg link button doesn't work) – I'm confident it's better to just select the text directly and keep the toolbars separate.

the link button is another issue, even without my pr, the link button is not working on table toolbar 😂.

Kapture 2021-07-16 at 12 46 26

JiangWeixian commented 3 years ago

maybe is able to implement it in this way

image

tommoor commented 3 years ago

This is a nice approach, but it does mean a reasonable amount of extra development for the extra popout positioning.

Another related note: It would be really nice to move the add column buttons above the columns like in this screenshot, it's the same in most other tools and would remove two buttons from the toolbar then

(Also I already cherry-picked the separator fix onto main btw)