Joystream / pioneer

Governance app for Joystream DAO
https://pioneerapp.xyz/
GNU General Public License v3.0
44 stars 70 forks source link

Forum: Editing Tables #4032

Closed traumschule closed 1 year ago

traumschule commented 1 year ago

CkEditor isn't able to help producing tables. However it can be used to save a a post including a correctly formatted table.

On editing that post however CkEditor breaks that table while it should just keep its raw format as it is.

table editing-table Thread ID: 2

dmtrjsg commented 1 year ago

This looks like a more complex issue than a 2, as it may require to convert the markdown in the CK editor format but it is unclear where exactly it breaks..

traumschule commented 1 year ago

For some reason we don't have the table feature.

The basic table feature is enabled by default in all predefined builds.

jen4ph commented 1 year ago

On editing that post however CkEditor breaks that table while it should just keep its raw format as it is.

Hi, just want to share, when I edit tables(done it multiple times) on Forum I do not encounter this issue. I am no expert but one thing that works for me is using GH when drafting tables and texts. Like, when we do council reports/plans, initially it is drafted in Notion/google docs, I copy-paste it first on GH then copy-paste again on Forum, when I edit I just follow the format it has, and works every time. I hope this makes sense.

Screen Shot 2023-03-18 at 3 06 26 PM Screen Shot 2023-03-18 at 3 05 45 PM
traumschule commented 1 year ago

For the post above i used this format:

| Day | Activity |  
|--|--|  
| 4 | Council [hired](https://pioneerapp.xyz/#/working-groups/openings/distribution-0) [l1dev](https://pioneerapp.xyz/#/members/515) as lead ([369,206](https://polkadot.js.org/apps/?rpc=wss://rpc.joystream.org:9944/ws-rpc#/explorer/query/369206)), f[irst worker opening](https://pioneerapp.xyz/#/working-groups/openings/distribution-2) ([371,917](https://polkadot.js.org/apps/?rpc=wss://rpc.joystream.org:9944/ws-rpc#/explorer/query/371917)) |  
| 5 | community-repo housekeeping ([#867](https://github.com/Joystream/community-repo/pull/867)) |  
| 6 | Hired 6 workers ([398,709)](https://polkadot.js.org/apps/?rpc=wss://rpc.joystream.org:9944/ws-rpc#/explorer/query/398709) |
jen4ph commented 1 year ago

I use this format

No. | Candidate | Link to Program
-- | -- | --
1 | Tomato | https://pioneerapp.xyz/#/forum/thread/262?post=1148
2 | 0x2bc | https://pioneerapp.xyz/#/forum/thread/162?post=1115
3 | Jen4ph | https://pioneerapp.xyz/#/forum/thread/22?post=1123
ivanturlakov commented 1 year ago

Tested on https://dao-git-dev-joystream.vercel.app/#/forum/thread/1 wss://54.205.24.16.nip.io/ws-rpc

⚠️ Input v1 |--|--|

Screenshot 2023-04-27 at 18 41 48

✅ Input v2 -- | -- | --

Screenshot 2023-04-27 at 18 41 57

⚠️ Adding table via editor tool

Screenshot 2023-04-27 at 18 42 07
dmtrjsg commented 1 year ago

@chrlschwb is it at all possible to address the last case "adding table via editor"? we may want to just take out Table from this release... please share your thoughts with @thesan when you have a chance..

chrlschwb commented 1 year ago

I've already found a fix to the issue. @thesan which branch should I push it to?

traumschule commented 1 year ago

I've already found a fix to the issue. @thesan which branch should I push it to?

IMO the best option would be to disable the table ui in a PR then to try re-enable is and try to solve it in a different PR. This way the first PR would be merged now and make it to the release while the 2de PR would make it to a future release. WDYT ?


what is the use of the "ReactMarkdown" component? src\common\components\MarkdownPreview\MarkdownPreview.tsx

It's the components which actually convert the markdown to html

The problem with the table in ckeditor arises from this ReactMarkdown component which accepts only string while the table is an object. This code should fix it but then we must abandon ReactMarkdown

Please review the file. I think ReactMarkdown component is not essential, afaik it only enhances styles. I can replace it with a different piece of code. But the ckeditor table plugin cannot be used in combination with that component.

AFAIK the code on your screenshot won't work if the markdown variable contains actual markdown like a [link](url)

That's true. What I'm suggesting is to remove the ReactMarkdown component and find different ways to enable markdown.

Replacing ReactMarkdown is an option but IMO it's not the easiest. You should know that there has been some work put in the rehype/remark plugins to extend standard markdown. Eg it supports member mentions (shows a popup with member infos). I think the effort should be into getting the ckeditor extension to output ghf markdown tables (just like what's already supported) instead of html. If the extension doesn't support this, some filters can be provided to change the way the ckeditor ast gets parsed into markdown. This is used for member mentions for example. But to clarify this is the part on the ckeditor side not react markdown (it's the markdown which is gets written on chain, not what's read from the QN) it was introduced back in May 2021: https://github.com/Joystream/pioneer/commit/94dcfcf44f1d5b4079f37b301c0b2211edf85a4c