Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/artciles: add softbreak plugin #306

Open
wants to merge 164 commits into
base: feature/articles
Choose a base branch
from

Conversation

maerzhase
Copy link
Contributor

@maerzhase maerzhase requested a review from ciphrd August 1, 2022 08:00
@maerzhase maerzhase self-assigned this Aug 1, 2022
@vercel
Copy link

vercel bot commented Aug 1, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
fxhash ✅ Ready (Inspect) Visit Preview Sep 19, 2022 at 10:44AM (UTC)

@GemN
Copy link
Contributor

GemN commented Aug 1, 2022

I think it's too permissive at the moment, you're not checking the node type, it could add undesired side effects, I would propose something like

  editor.insertSoftBreak = () => {
    // check the node type is p / ul / ol etc
    // insert \n
    insertSoftBreak() // to allow other plugins to work if they manipulate soft break
  }

@maerzhase
Copy link
Contributor Author

maerzhase commented Aug 2, 2022

sure we can add that, i am not sure if its necessary though. softlinebreak is only triggered on text nodes. that should already exclude all custom elements.

but we can also retrieve the node by cursor position and do a check for the type.

@maerzhase
Copy link
Contributor Author

I included the changes proposed by @GemN 👍

Copy link
Collaborator

@ciphrd ciphrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's good to check for the block type, as we want to avoid any weird side effect.

There is an issue in the markdown transformation, this is the string which gets computed:

This is a test 
We can have multiline now, great

Another test

(see here https://gateway.fxhash-dev2.xyz/ipfs/Qmcfk4M1jwkFiN3FhEw1m1PoZX5nqjSPKqGHxuFKAM7qgV)

Resulting in the viewer not displaying the multiline properly:

image

I believe the resulting markdown should be:

This is a test
We can have multiline now, great

Another test

This is by default rendered like that by markdown viewers:

This is a test We can have multiline now, great

As there is no support for backline in paragraphs. It needs to be added.

@maerzhase
Copy link
Contributor Author

the encoding comes from the mdast-util-to-markdown: syntax-tree/mdast-util-to-markdown@108450c

when i started to replace the encoded whitespaces it also breaks the newlines. @ciphrd i am not sure how to resolve this issue

GemN and others added 28 commits September 14, 2022 13:52
…it goes to article edit page, bin delete the local changes
…ettier

Revert "Add eslint plugin prettier"
fix(article): update media size (video, image)
fix(article): autosave warn user if using incognito mode, check that an article saved correctly, check localStorage is available
fix(article): embed youtube need allow-presentation to work properlly
fix(article): change article li style
fix(article): media video dont crash page when url is wrong
fix(article): thumbnail update correctly, don't throw error on edition
fix(article): card nft article have buttons working as intended -> edit goes to article edit page, bin delete the local changes
fix: optimize infinite scroll - doesn't trigger empty queries when under 20 results
Bring prettier back due to improper merge
@maerzhase maerzhase changed the title add softbreak plugin Feature/artciles: add softbreak plugin Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Article editor] SHIFT + ENTER should insert a new line, and not insert a new block
4 participants