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

Read email content for each email in Thread #23

Merged
merged 7 commits into from Aug 25, 2021

Conversation

dab246
Copy link
Member

@dab246 dab246 commented Aug 13, 2021

  • Implement get email content when click each email
  • Mark unread email, flagged email in list email
  • Display message content with text/plain format
  • Display address To, Cc, Bcc header with Collapse/Expand mode
  • Display message content with text/html format
Screen.Recording.2021-08-13.at.18.25.32.mov

Demo Display email content with html format

content_html.mov
Screen.Recording.2021-08-19.at.18.12.55.mov

@chibenwa
Copy link
Member

Looks good so far.

Tiny remarks:

  • Inlined attachment should not be rendered as attachments: exclude them from attachment list
  • I'm slightly concered about display when we have say 20+ attachments: they would take all of the space.
    • I would recommand that when we have 4 or + attachments we only display 20 attachments (expend button) (mobile, on tablet today behaviour likely is acceptable.)
  • Be sure you do display Cc and Bcc headers if present on the detailed email view.

Use sentDate for the date display in email detailed view and email list view.

@chibenwa
Copy link
Member

Using Mailbox/query you can be able to load the mailbox list in parallel to the email list: this will fasten the loading time.

(I have the impression you load the email list then the mailbox list...)

@chibenwa
Copy link
Member

One other display remark: it is hard for me to distinguish from the email list read emails from unread emails.

Let me suggest you to render the subject in bold when the email is unread, normally when it is read, this might be enough to be doing the trick.

@chibenwa
Copy link
Member

Do we support display of Important / flagged messages ? IE the Star ?

You can star messages in OpenPaaS and try to support that as well in TMail.

@chibenwa
Copy link
Member

(And of course I trust @hoangdat or prioritizing my remarks :-) )

@Arsnael
Copy link
Member

Arsnael commented Aug 13, 2021

Wow, impressive! Good job guys, looks damn great!

@chibenwa
Copy link
Member

chibenwa commented Aug 13, 2021

I am not convinced of the folding of To, Cc Bcc... I would prefer to have it always expended to be honest.

(At least when it fits in 3-4 lines.)

It is important information and in the detailed email view we have room. We likely should use it.

@dab246
Copy link
Member Author

dab246 commented Aug 16, 2021

I am not convinced of the folding of To, Cc Bcc... I would prefer to have it always expended to be honest.

(At least when it fits in 3-4 lines.)

It is important information and in the detailed email view we have room. We likely should use it.

What do you mean? Please be more specific. Can you draw detailed pictures so I can understand you.

@chibenwa
Copy link
Member

THis one is cool:

cool

This one can be improved (display of important information takes extra clicks while it would not harm the display...

Screenshot from 2021-08-16 09-03-59

}

if (unreadThreads == null || unreadThreads!.value.value <= 0) {
Copy link
Member

Choose a reason for hiding this comment

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

IMO, unreadEmails is better.

<application
android:name="io.flutter.app.FlutterApplication"
Copy link
Member

Choose a reason for hiding this comment

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

Why not TMail?

@dab246 dab246 force-pushed the get_email_content branch 4 times, most recently from 472cb90 to b4b7940 Compare August 18, 2021 10:51
@chibenwa
Copy link
Member

Can you update the video now that text/html is supported? <3

@dab246
Copy link
Member Author

dab246 commented Aug 18, 2021

Can you update the video now that text/html is supported? <3

Please see the video I attached above. It's not complete yet, I'm testing and fixing some bugs.

@chibenwa
Copy link
Member

Huge thanks and nice work!

@chibenwa
Copy link
Member

Remember to get rid of John Doe // user@exemple.com on the top of the mailbox view ;-)

import io.flutter.embedding.engine.FlutterEngine
import io.flutter.plugins.GeneratedPluginRegistrant

class MainActivity: FlutterFragmentActivity() {
Copy link
Member

Choose a reason for hiding this comment

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

why we need to create this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

run android

Copy link
Member

Choose a reason for hiding this comment

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

But you delete another MainActivity. Please help me to understand more in one comment, no need to save the time with short sentences.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I renamed packagegage, git saved it like that.

pubspec.yaml Show resolved Hide resolved
pubspec.yaml Outdated Show resolved Hide resolved
pubspec.yaml Show resolved Hide resolved
@dab246
Copy link
Member Author

dab246 commented Aug 23, 2021

NOTE

  • Some Emails with content in the form of a full web page have not been displayed, including full tags such as:
<!DOCTYPE html>
<html>
<head>
<title>Page Title</title>
</head>
<body>
</body>
</html>

Specific email address:
noreply@qa.open-paas.org (Web: Dislplay | Mobile, Tablet: Not display)
robin@thebusinessanalystjobdescription.com

REASON

  • Many tags are formatted with css properties that cannot be parsed by the library resulting in nullable objects.

@chibenwa
Copy link
Member

Many tags are formatted with css properties that cannot be parsed by the library resulting in nullable objects.

As a safe guard can we sanitize such CSS properties before the display?

Did you reported the bug upstream?

Do we have other HTML libraries to try out?

@dab246
Copy link
Member Author

dab246 commented Aug 23, 2021

Many tags are formatted with css properties that cannot be parsed by the library resulting in nullable objects.

As a safe guard can we sanitize such CSS properties before the display?

Did you reported the bug upstream?

Do we have other HTML libraries to try out?

  • If you delete some important css, it will affect the display interface.

  • I tried the two most popular libraries on pub dev, but the results are still not satisfactory. I gave them feedback to fix the problem. Meanwhile I'm trying to get their project on git to debug and fix the problem.

  • If possible can you give me the code for handling this display on the font-end side of the web team

@chibenwa
Copy link
Member

If possible can you give me the code for handling this display on the font-end side of the web team

cc @fabienmoyon ?

@fabienmoyon
Copy link
Member

@dab246 and @chibenwa for this we used https://github.com/cure53/DOMPurify like others projects like Protonmail ;)

@hoangdat
Copy link
Member

@dab246 and @chibenwa for this we used https://github.com/cure53/DOMPurify like others projects like Protonmail ;)

It quite complicated, and IMO, we should create an issue and move it to the relevant sprint to solve that. We still have an useable reading mail widget. WDYT? @dab246 @chibenwa

@hoangdat
Copy link
Member

btw, how about it: https://pub.dev/packages/sanitize_html

@dab246
Copy link
Member Author

dab246 commented Aug 23, 2021

@dab246 and @chibenwa for this we used https://github.com/cure53/DOMPurify like others projects like Protonmail ;)

I sanitizing the HTML content and it showed up. But it has removed some of the styles of the tags so the interface is not displayed correctly. How did you handle it after sanitizing the HTML @fabienmoyon

@dab246
Copy link
Member Author

dab246 commented Aug 23, 2021

Issues: #51

@fabienmoyon
Copy link
Member

I sanitizing the HTML content and it showed up. But it has removed some of the styles of the tags so the interface is not displayed correctly. How did you handle it after sanitizing the HTML @fabienmoyon

@dab246 Yes you need to authorize some CSS and HTML tag like https://github.com/linagora/esn-frontend-common-libs/blob/ffdd13b42ca611d480f90761a879b0166348ba06/src/frontend/js/modules/core.js#L235

@dab246 dab246 requested a review from hoangdat August 24, 2021 10:28
@hoangdat hoangdat merged commit 4afb309 into linagora:main Aug 25, 2021
@fabienmoyon
Copy link
Member

One more thing to this issue, we will also use https://github.com/Automattic/juice to be able to inline style tag in the email body, like this we will be able to display the expected result and to not impact our style

@hoangdat
Copy link
Member

hoangdat commented Oct 8, 2021

One more thing to this issue, we will also use https://github.com/Automattic/juice to be able to inline style tag in the email body, like this we will be able to display the expected result and to not impact our style

so cool, we should investigate more.

btw, @fabienmoyon please download our application, try to use to read email, and feedback to us, may be give us more idea for html rendering https://app.bitrise.io/artifact/94688104/p/edd9b9adf8b0e1d30e537108230dc096

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.

None yet

5 participants