Integrate the attachment block markers into the attachment templates
ClosedPublic

Authored by vkrause on Oct 3 2017, 12:31 PM.

Details

Summary

This further simplifies attachment container Grantlee template usage.

Diff Detail

Repository
R94 PIM: Message Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
vkrause created this revision.Oct 3 2017, 12:31 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptOct 3 2017, 12:31 PM
Restricted Application added a subscriber: KDE PIM. · View Herald Transcript
knauss added a comment.Oct 3 2017, 1:34 PM

Also to do this with the root block would add a lot of global logic into the grantlee themes. Maybe we find a better solution for this.

messageviewer/src/messagepartthemes/default/templates/asiconpart.html
5

not really happy with this change, becuase it makes it harder to read. and mire error prune to forget a clossing "</div>".
Also maybe we want this logic to be done globally, to deside, ok this is an attachment, so we add this block to the part. IMO it doesn't makes sense to add this to every plugin. And to make the links usefull for kmail to jump to attachments we need these links.

vkrause added inline comments.Oct 3 2017, 1:52 PM
messageviewer/src/messagepartthemes/default/templates/asiconpart.html
5

Well, this change moves the burden of adding the attachment blocks into the Grantlee templates and out of the individual plugins. This is what makes the current code of D8122 work without requiring any handling of isAttachment(). I think that is a step in the right direction.

It might indeed be something we can add centrally in DefaultRenderer, and possibly even make non-conditional (having this for non-attachments doesn't really hurt after all), but that's a further improvement IMHO.

knauss accepted this revision.Oct 3 2017, 2:08 PM
knauss added inline comments.
messageviewer/src/messagepartthemes/default/templates/asiconpart.html
5

Well, this change moves the burden of adding the attachment blocks into the Grantlee templates and out of the individual plugins. This is what makes the current code of D8122 work without requiring any handling of isAttachment(). I think that is a step in the right direction.

yepp you are right +1

This revision is now accepted and ready to land.Oct 3 2017, 2:08 PM
This revision was automatically updated to reflect the committed changes.