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

Editor Changes #507

Merged
merged 20 commits into from
Nov 9, 2023
Merged

Editor Changes #507

merged 20 commits into from
Nov 9, 2023

Conversation

DarrachBarneveld
Copy link
Contributor

โœจ Codu Pull Request ๐Ÿ’ป

Codu Logo

๐Ÿ‘‰ Please remove the below and replace with your own values, leaving the headers where they are. ๐Ÿ‘ˆ

Pull Request details:

Added target blank and code highlight styling changes. Having issues rendering styled code to preview post. Install lowlight/highlight packages

Any Breaking changes:

None

Assosiation with issue #498 and #497

@vercel
Copy link

vercel bot commented Oct 9, 2023

@DarrachBarneveld is attempting to deploy a commit to the Codรบ Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 9, 2023

The latest updates on your projects. Learn more about Vercel for Git โ†—๏ธŽ

Name Status Preview Comments Updated (UTC)
codu โœ… Ready (Inspect) Visit Preview ๐Ÿ’ฌ Add feedback Nov 9, 2023 7:26am

))}
</select>

<pre className="bg-black border ">
Copy link
Contributor

Choose a reason for hiding this comment

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

@NiallJoeMaher are there styles for code blocks at the moment? may be easier to add here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Happy to deal with the smaller style issues at the end. Its not super important for alpha page

Copy link
Contributor

Choose a reason for hiding this comment

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

The styles we currently have are great; I wonder if we can mimic or what people think ๐Ÿค”

Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to - We can add this and figure it out - if it doesnt work we can revert the changes and save syntax highlighting for v2 of the editor

import { ChangeEvent, FunctionComponent } from "react";

// Change this for code styling
import "highlight.js/styles/monokai-sublime.css";
Copy link
Contributor

Choose a reason for hiding this comment

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

changes look good, check with @NiallJoeMaher re styling of the code blocks. Good job.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had quite a lot of difficulty getting the syntax highlighter to work in preview/render mode. Will take another look now

Copy link
Contributor

Choose a reason for hiding this comment

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

https://tiptap.dev/guide/node-views#render-html

you can extend the configuration for the node in tiptap extensions - . eg Codeblock.configure{renderHTML({ HTMLAttributes }) {
return ['my-custom-node', mergeAttributes(HTMLAttributes)]
},}

Its not essential now, we can fix in another pr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try this, I went another way thats probably not ideal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is its parses as a different node. The StarterKit.Codeblock one, not the customised one, ๐Ÿค”

Copy link
Contributor

Choose a reason for hiding this comment

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

you should be able to configure all of that. But If what you have works, then we can add it. I don't mind to fix styles up at the end.

Copy link
Contributor

Choose a reason for hiding this comment

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

technically everything can be target in global.css with .ProseMirror code, .ProseMirror p etc to make the styles sync - we can figure it out later if you wish, one source of truth should be feasable in the editors case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have two solutions. Maybe I should PR the alternate also, each have pros and cons I think?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth chatting about it in the Discord too just so we can discuss too!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, will log on this evening ๐Ÿ‘

@NiallJoeMaher
Copy link
Contributor

Let me know if you are going to fix the conflicts and we can add it in case we need backward compatibility.

@NiallJoeMaher
Copy link
Contributor

If you can would you fix the conflicts and I'll get this merged! I also need to send you the Codรบ Hacktoberfest badge ๐Ÿฆก

@NiallJoeMaher NiallJoeMaher mentioned this pull request Nov 7, 2023
@DarrachBarneveld
Copy link
Contributor Author

OMG! A collectible badge, my weakness. Will fix conflicts now ๐Ÿ˜ƒ

Copy link
Contributor

@NiallJoeMaher NiallJoeMaher left a comment

Choose a reason for hiding this comment

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

LGTM! ๐ŸŒฎ

@NiallJoeMaher NiallJoeMaher merged commit 9c27968 into codu-code:develop Nov 9, 2023
1 check passed
@DarrachBarneveld DarrachBarneveld deleted the editor branch November 9, 2023 09:41
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.

3 participants