-
Notifications
You must be signed in to change notification settings - Fork 71
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
Integrate Nostr-Editor #445
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The images are loaded and displayed inside the editor, as a result, the Compose component replace the ImageInput component. The goal is still to keep the underline carrousel, I miss a proper callback to get the uploaded image urls (onComplete not triggering)
I'm not sure why onComplete wouldn't trigger, you may have to debug that in nostr-editor. Let me know if you'd like me to take a look. I do think we can potentially get rid of the image carousel at the bottom and put everything inline. I'd like the editor to show pretty much the same thing as would ultimately be rendered.
When hitting Enter to select a mention, the editor does break the line, preventDefault does not help
This is probably due to the use of onKeyDown in a way that's not integrated with tiptap. Take a look at how flotilla does mention suggestions via a tiptap plugin, you'll probably have to do something similar.
tags or hashtags are immediately wrapped in tags, it is underlined as you type before "space" key is hit
I haven't been able to figure out topics. I was able to avoid solving this in flotilla, but it does need to be done. Maybe ignore nostr-editors TagExtension and add a new Node type.
nostr-editor comes with a nostr event parsing. Images url are immediately set in the content and in the tags. We could potentially use this functionality entirely and remove internal logic from Coracle, but it depends how confident you are with this dependency.
Yep, rip out as much of coracle's code for this and start from scratch. It's outlived its usefulness, although we'll have to make sure the (many) bugfixes make it into the new version. I don't mind the dependency on tiptap, it's what's necessary to bring the UX to the next level.
Cesar and I had also talked about using tiptap to render notes. He was apparently able to get that working, but I've had some trouble. Anyway, I'm not sure it would be performant in practice so we probably shouldn't do it right away. Something to keep in mind though.
As I said before, feel free to nuke NoteImages, Content, NoteReply, NoteCreate, and NoteCreateInline (etc) and come up with new abstractions. I think it's going to be better to tackle the problem completely from scratch.
src/app/shared/Compose.svelte
Outdated
}, | ||
|
||
extensions: [ | ||
StarterKit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StarterKit has lots of stuff we don't want to support. See flotilla's set up for something much closer to reality.
src/app/shared/Compose.svelte
Outdated
if (["Enter"].includes(e.code) && suggestions.get()) { | ||
e.preventDefault() | ||
const pubkey = suggestions.get() | ||
if (pubkey) { | ||
const content = $editor.getHTML() | ||
// only replace the last occurence | ||
const replacedContent = content.replace(/(.*)@\w+/, "$1") | ||
$editor.commands.setContent(replacedContent, true) | ||
$editor.commands.insertNProfile({nprofile: pubkeyEncoder.encode(pubkey)}) | ||
suggestions.setData([]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach will probably not work, tiptap is too magical, you have to do things their way.
{#if node} | ||
<NodeViewWrapper data-type="tag" as="span"> | ||
<Anchor modal underline on:click={onClick}> | ||
#{value} | ||
</Anchor> | ||
</NodeViewWrapper> | ||
{:else} | ||
<Anchor modal underline on:click={onClick}> | ||
#{value} | ||
</Anchor> | ||
{/if} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't overload NoteContentTopic and PersonLink, just create new components specifically for the editor. Again, see flotilla for how I've structured it there.
Took a lot from Flotilla in this commit. Created a new folder "editor" under the app directory, like Flotilla, but with different Edit component. Let me know if you would rather have it somewhere else, or just add the Edit[whatever].svelte files in the shared directory.
|
Great, looking good.
Yeah, I think the reason it's this way is so that submit is deferred until files get uploaded. As a result, we currently have to defer uploading files until the user submits the form (which is a feature in a way, because it avoids them uploading files they don't ultimately want to use). I do think that uploading files concurrently for a quick submit is a better way to do this though, so hopefully we can find a way around this pattern. |
Issues:
|
That might be the content security policy, see
I've struggled with this as well. @cesardeazevedo, do you happen to know the solution to this?
|
Not so sure about the actual issue here but the tag extension was very buggy before version 0.0.3.
I feel like the editor still struggles with things like hardbreak lines, I have found difficult to understand the correct approach.
I would imagine the nostr-editor to be considerably slower than
Nostr-editor indeed needs to provide an easy way to access the uploaded files, gonna work on that.
This part indeed needs some improvement.
Not so sure if I understood correctly, but you can't directly set a event content when initializing the editor const editor = useEditor({
content: 'Hello nostr:nprofile1qy88wumn8ghj7mn0wvhxcmmv9uq32amnwvaz7tmjv4kxz7fwv3sk6atn9e5k7tcprfmhxue69uhhyetvv9ujuem9w3skccne9e3k7mf0wccsqgxxvqas78x0a339m8qgkaf7fam5atmarne8dy3rzfd4l4x6w2qpncmfs8zh',
}) This will render as text as tiptap won't apply any pasteRules here, I tried to find a way many times but couldn't, you have to call onCreate({ editor }) {
editor.commands.setEventContent({
id: '...',
content: 'Hello nostr:nprofile1qy88wumn8ghj7mn0wvhxcmmv9uq32amnwvaz7tmjv4kxz7fwv3sk6atn9e5k7tcprfmhxue69uhhyetvv9ujuem9w3skccne9e3k7mf0wccsqgxxvqas78x0a339m8qgkaf7fam5atmarne8dy3rzfd4l4x6w2qpncmfs8zh',
})
} Interestingly you can set a HTML representation of the editor state, which is the content you get when copying the content directly from the editor (check the clipboard inspector) const editor = useEditor({
content: `<meta charset='utf-8'><p data-pm-slice="1 1 []"><span nprofile="nostr:nprofile1qy88wumn8ghj7mn0wvhxcmmv9uq32amnwvaz7tmjv4kxz7fwv3sk6atn9e5k7tcprfmhxue69uhhyetvv9ujuem9w3skccne9e3k7mf0wccsqgxxvqas78x0a339m8qgkaf7fam5atmarne8dy3rzfd4l4x6w2qpncmfs8zh" pubkey="c6603b0f1ccfec625d9c08b753e4f774eaf7d1cf2769223125b5fd4da728019e" relays="wss://nos.lol/,wss://relay.damus.io/,wss://relay.getalby.com/v1" data-type="nprofile">@</span> </p>`
}) |
It seems to be solved now by instantiating the editor with the content as HTML.
Have finally managed to have tweet preview working while keeping the content security policy, that's 2 twitter script tag, once being labelled with a git tag, but it works
Happy to get your feedback on the editor, until then I'll just keep merging dev and move on to other issues |
thanks @cesardeazevedo for your answer ! I managed to have it all working well with file upload, I will keep testing and issue a PR on nostr-editor if needed. I did have that strange issue where file attributes where not reported correctly when using immediate uploads, the examples on nostr-editor repo works as expected, but with the Coracle config, it consistently updated the wrong node, the one right before the actual Image node (pos - 1). |
Please do, it's worth having upstream.
Try https://blossom.hzrd149.com/
Yeah, let's do it. -- I'll test the PR, can you rebase from dev to fix the conflicts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to make you undo a fair amount of work, but I don't think it's necessary to render everything as blocks. WYSIWYG is nice in theory, but since we're using a different renderer anyway, we're going to have differences between compose/render. Take a look at how flotilla handles things like links and entities. Pretty much everything should be inline in the editor, and not super rich.
Also, compare the behavior of enter between production and this PR, it's important that enter/shift+enter/meta+enter stay the same. I was testing it out and published a dumb note accidentally because of the change. Try to get the design looking the same too, in terms of colors, textarea height, etc. A lot of finessing has gone into what we currently have.
My understanding is that mod-enter is to submit, enter and shift-enter is for hardbreak. It is now the default configuration for the editor.
Have been through all the Compose component, styles should be aligned. There is one annoying style bug, the animation when you open a reply, it goes in 2 phase if you pay attention to the cursor, have spent a fair amount of time getting this right, no luck so far. I'll sleep on it |
It is in a chat context, but it's actually the reverse when composing a note. Sort of weird, but seems the most intuitive. |
Been hitting cors for any endpoint I tried, also https://void.cat. |
src/app/editor/FileUpload.ts
Outdated
this.editor.storage.fileUpload.tags.push([ | ||
"imeta", | ||
`url ${res.size}`, | ||
`size ${res.size}`, | ||
`m ${res.type}`, | ||
`x ${res.sha256}`, | ||
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blossom do not seems to enforce a standard for meta tags in the response, I'm taking all useful fields in the default response to build a meta tag.
81888de
to
5a920e8
Compare
Something is very wrong with some of these PRs, try checking dev out and cherry-picking your changes, then doing Also, maybe try https://blossom.f7z.io/ for blossom? |
I rebased from origin/dev indeed, I had to go through twenty-ish commits cherrypicking the right changes, I might be doing something wrong, I'm not very used to rebase |
It's the reverse, the diff is including changes that already exist on dev. This is likely due to me force-pushing to dev at some point in the past after a merge (which is why rebase works better). Try rebasing with |
I'm happy to hop on a call and work on it with you if that would be helpful |
af81dcc
to
8cae299
Compare
9353b32
to
0cc2804
Compare
src/app/editor/FileUpload.ts
Outdated
} catch (error) { | ||
console.error(error) | ||
this.editor.storage.fileUpload.loading.set(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this bit, finally will still run if an error happens
|
||
if (!content) return showWarning("Please provide a description.") | ||
|
||
if (!skipNsecWarning && content.match(/\bnsec1.+/)) return nsecWarning.set(true) | ||
|
||
const tags = [...tagsFromContent(content), ...getClientTags()] | ||
const tags = [...tagsFromContent(content), ...getClientTags(), ...editor.commands.getMetaTags()] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like tagsFromContent
could be replaced by nostr-editor's new tags functionality, or at least getEditorTags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cesar recently started working on it, it probably isn't ready yet.
getEditorTags will gives you all #whatever in the editor. It refers to hashtags not nostr event tags
<i slot="button" class="fa fa-paperclip cursor-pointer" /> | ||
</ImageInput> | ||
<button | ||
class="hover:bg-white-l staatliches flex h-7 w-7 cursor-pointer items-center justify-center gap-2 whitespace-nowrap rounded bg-white px-6 text-xl text-black transition-all" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This button should look like the others. Also, be careful about using black
and white
, which won't play well with light theme.
8cae299
to
5fca98b
Compare
|
5fca98b
to
ee6dc9d
Compare
CORS blocked too, only tried from localhost, it might be it |
bbf1346
to
18e30e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting there!
- There is a difference in newline rendering between edit and render (see image below). This is probably just a matter of using
this.editor.getText({blockSeparator: "\n"})
when calling getText. - https://github.com/coracle-social/coracle/pull/445/files#r1824731976
- For some reason, mentions don't work right after topics (see video below)
Screen.Recording.2024-11-07.at.2.00.26.PM.mov
I also ran into this error when pasting a nevent
:
proxy.js:385 Uncaught TypeError: Cannot read properties of undefined (reading 'length')
at Array.displayEvent (EditEvent.svelte:16:20)
at Array.create_default_slot (EditEvent.svelte:36:18)
at create_slot (utils.js:165:22)
at create_dynamic_element (EditorContent.svelte:34:24)
at create_fragment2 (NodeViewWrapper.svelte:13:11)
at init (Component.js:148:34)
at new NodeViewWrapper (NodeViewWrapper.svelte:14:21)
at create_fragment (EditEvent.svelte:35:65)
at init (Component.js:148:34)
at new EditEvent (EditEvent.svelte:27:14)
Pasting npubs is also buggy:
Screen.Recording.2024-11-07.at.2.03.57.PM.mov
Background for link-content looks bad in onboarding (as well as in replies and chat):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged 🎉 although I have a couple small changes you can follow up on
Have the note creation to use nostr-editor, a set of nostr extension on top of the tiptap editor.
Bugs and known issue remaining: