-
Notifications
You must be signed in to change notification settings - Fork 263
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
Sets <p> margin to 0 for sent emails #2538
Conversation
c07b222
to
8ced49c
Compare
I've tried to create a ckeditor plugin to implement this functionality. Without success till now. If someone @nextcloud/mail with knowledge about ckeditor's internals could review my PR to try and understand what should be done ,that would be great. ping @ChristophWurst |
8bb5bec
to
01b0788
Compare
This should be working properly now Cyrille |
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.
Looks very clean 👍 Thanks a lot!
src/components/Composer.vue
Outdated
@@ -342,7 +342,9 @@ export default { | |||
bcc: this.selectBcc.map(this.recipientToRfc822).join(', '), | |||
draftUID: uid, | |||
subject: this.subjectVal, | |||
body: this.editorPlainText ? htmlToText(this.bodyVal) : this.bodyVal, | |||
body: this.editorPlainText |
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.
Works, but from a logical PoV the composer shouldn't have to care about this. This should be moved to the TextEditor.vue
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.
Makes sense but I can't figure out how to implement this there.
What could probably easily be done would be to add a "style" attribute to all <p>
elements rather than assign them a class:
viewWriter.addClass('null-margin', conversionApi.mapper.toViewElement(data.item))
might be changed to add an attribute rather than adding a class.
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.
Makes sense but I can't figure out how to implement this there.
Possibly at
mail/src/components/TextEditor.vue
Line 141 in 1691463
this.text = this.value |
style
tag. I also just realized that when we only add the style element when the message is sent, the previewed message looks different than what is actually sent.
What could probably easily be done would be to add a "style" attribute to all
<p>
elements rather than assign them a class:
If that is easy to do I'm fine with that as well :) This will also show a correct preview of what will be sent (independent of Nextcloud's paragraph styling)
Can't test right now as I have my local setup migrated to #2064 but will give it a test once that PR is merged :) |
bb24284
to
78ed1cc
Compare
This works fine with the rich text editing mode. But the plain text one still has line breaks for paragraphs, no? |
I think it's fixed now |
Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
function so that they get converted to a single newline only. Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
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.
tested and it works
ddcd33e
to
bb88284
Compare
fixes #2481 and #2513 I guess )
Signed-off-by: Cyrille Bollu cyrpub@bollu.be