View raw message source

[StCyr]

Should fix #2221

TODO:

• Effectively show raw message (with modal component?)
• Add a nice icon
• Also add action in Message component
• Return raw message as html rather than json

[ChristophWurst]

I'll try to find some time to look into this and see where we should put the logic. Hacking it into the existing already quite complex logic won't be the best choice in the long run.

#2214 and #2064 will clean some of that logic up. We might want to wait with this feature until these large features are done 🙌

Hope this makes sense ✌️

[jancborchardt]

Design-wise:

• Should only be in the 3-dot menu when in the mail detail view, last item before "Delete".
• Wording "View source"
• Icon can be .icon-details (the info i)

[StCyr]

Design-wise:

* Should only be in the 3-dot menu when in the mail detail view, last item before "Delete".

* Wording "View source"

* Icon can be `.icon-details` (the info i)

Fixed

[jancborchardt]

Just a note @StCyr, since you are member of our organization on Github, you can also just work on branches of the main repository and don’t need a fork. :) This has the advantage that if any changes are needed or you want to hand over a pull request, other contributors can continue.

[StCyr]

I'll try to find some time to look into this and see where we should put the logic. Hacking it into the existing already quite complex logic won't be the best choice in the long run.

#2214 and #2064 will clean some of that logic up. We might want to wait with this feature until these large features are done 🙌

Hope this makes sense ✌️

I let you devise if some refactoring is needed. But, for me, it's done.

[ChristophWurst]

Just a note @StCyr, since you are member of our organization on Github, you can also just work on branches of the main repository and don’t need a fork. :) This has the advantage that if any changes are needed or you want to hand over a pull request, other contributors can continue.

That's not true anymore. For a long time they changed this and you can now (unless explicitly opted out) push to the PR's fork branch as a maintainer of the destination repo. I use this regularly to clean up external contributor's branches. It works just fine adding the other user's fork as new git remote and pushing there.

[jancborchardt]

It works just fine adding the other user's fork as new git remote and pushing there.

Still much more complicated than a simple branch change. ;) All about removing the small barriers.

[jancborchardt]

Nice! :) For me just 2 things are missing:

• "View source" entry should be before "Delete", as Delete should always be the last element in an action menu.
• Can we get the source displayed in a modal instead of a separate window and all. Not so important but would be nice. :)

[StCyr]

* [x]  Can we get the source displayed in a modal instead of a separate window and all. Not _so_ important but would be nice. :)

I've done this but Modal components don't have a vertical scrollbar and as such can't display the whole content :-(

[jancborchardt]

* [x]  Can we get the source displayed in a modal instead of a separate window and all. Not _so_ important but would be nice. :)

I've done this but Modal components don't have a vertical scrollbar and as such can't display the whole content :-(

Oh they don’t? I guess only the filepicker component has that. But the modal component should for sure have that. Could you try adding overflow-y: scroll; manually to the relevant container? Let me know if I should look into it instead. :)

Also cc @skjnldsv @juliushaertl on how we could do this properly in the component – should be possible of course to have modals with scrollbar?

[juliusknorr]

Also cc @skjnldsv @juliushaertl on how we could do this properly in the component – should be possible of course to have modals with scrollbar?

The idea is that the modal size fits to the content size so it is more flexible to be used with a fixed content size like in the image viewer. Adding the scollbars with CSS in the apps scope is fine I'd say.

[jancborchardt]

The idea is that the modal size fits to the content size so it is more flexible to be used with a fixed content size like in the image viewer. Adding the scollbars with CSS in the apps scope is fine I'd say.

Right, I’m also thinking about e.g. the keyboard shortcuts list and the account settings. Both of these would work a bit better in a modal than the display they use right now, with left navigation still visible. But not fully relevant to this pull request, will open an issue in the components repo.

[StCyr]

Oh they don’t? I guess only the filepicker component has that. But the modal component should for sure have that. Could you try adding overflow-y: scroll; manually to the relevant container? Let me know if I should look into it instead. :)

it seems that the component applies a overflow: hidden style that overrides my settings:

[jancborchardt]

it seems that the component applies a overflow: hidden style that overrides my settings:

That’s because the style in the component is more specific as it has 2 classes. :) Simply change your rule to .modal-wrapper .modal-container so it has 2 classes as well. If that still doesn’t work (cause of the data attribute maybe), it’s fine in this case to use overflow-y: scroll !important; to override it.

Oh and yeah, only use overflow**-y** as we don’t want horizontal (-x) scrolling. :)

[StCyr]

If that still doesn’t work (cause of the data attribute maybe), it’s fine in this case to use overflow-y: scroll !important; to override it.

Yeah, indeed, I think the data attribute came into play during the tests I made.

I'll try to make use of that !important stanza then

[StCyr]

Ok, good for me

[ChristophWurst]

#2214 and #2064 will clean some of that logic up. We might want to wait with this feature until these large features are done raised_hands

Still blocked. But these two will be integrated soon. Will take a look at this one ASAP

[StCyr]

You can use getFullMsg in MessageMapper, though :)

Then my latest update to this PR might be good now :)

…

Le 10 févr. 2020 à 17:40, Christoph Wurst ***@***.***> a écrit : You can use getFullMsg in MessageMapper, though :)

[ChristophWurst]

I'm not sure, tbh. I tried to read the diff. But something is wrong with the commits. Could you please try to clean that up, so it's just one commit with your changes? :)

You moved the method from the message to the mapper, but the logic was unchanged. IMO it should be one dedicated method for the message source. We should not hack this into any other method. And the returned value shouldn't be an IMAPMessage but just a string, no?

Also, in the controller you don't have to catch all possible exceptions. It's fine to let some bubble up with the controller method is annotated with @TrapError. In Mail this means we'll catch the exception anyway, log it and return a proper JSON response.

[StCyr]

HI @ChristophWurst

I think I've corrupted my mail cache. Is there some command to reset it?

[ChristophWurst]

Looks like it lost recipients? What happens if you refresh the page?

[StCyr]

Looks like it lost recipients? What happens if you refresh the page?

Refreshing the page doesn't help :)

bah. It's okay now. I only have 2 more interesting emails left in this state.

[ChristophWurst]

So it was just a temporary bug? Or what do you mean by that?

Just to be clear: it's possible to clear the cache by dropping some data. But I'd really like to identify the problem first as this should be fixed before the release 😬

[StCyr]

So it was just a temporary bug? Or what do you mean by that?

Oh, yes, I think it was due to a change I made while working on this PR. Change that I have now reverted.

Just to be clear: it's possible to clear the cache by dropping some data. But I'd really like to identify the problem first as this should be fixed before the release 😬

So, no need to clear the cache anymore, for what I'm concerned. Thanks :)

[ChristophWurst]

Awesome 🙏

[StCyr]

@ChristophWurst

I think you might like this PR better now :)

[ChristophWurst]

Code looks indeed fine :) Just a few more nitpicks :)

[ChristophWurst]

Pushed a few minor fixes

• Remove use of dangerous v-html and use <pre> instead
• Show a loading spinner whlie the messages is fetched (this takes time for large messages or ones with attachments)
• Rename raw message to message source in the back-end
• Minor code cleanups

[ChristophWurst]

Let's do this :)