UksusoFF / webtrees-reminder

webtrees module for sending daily anniversaries list
20 stars 2 forks source link

Ugly text/plain part #23

Open slavkoja opened 3 years ago

slavkoja commented 3 years ago

Hi, the text/plain part of email is constructed via strip_tags, which is not bad at all, but this preserves all whitespaces and line ends, which results terrible readable text part:

    28 marca

                    Narodenie:
        Name Hidden

                            (1989 / 32)

This can be solved by some external library, which can be overkill. More simplest way can be to format the HTML to not have useless white spaces and line ends, for now, the HTML part (only related part)look as this:

<strong>28 marca</strong>
<br/>
                Narodenie:
    <a href="..."><span class="NAME" dir="auto" translate="no">Name <span class="SURN">Hidden</span></span></a>

                        (<span class="date">1989</span> / 32)
            <br/>
    <hr>
UksusoFF commented 3 years ago

I didn't think anyone else was using plain text in emails. Will check it but not now.

Also pull request welcome :)

slavkoja commented 3 years ago

Anyone, who cares about own privacy uses plain text at least as default or even disables HTML at all...

I tried two html2text libraries https://github.com/mtibben/html2text and https://github.com/soundasleep/html2text both are able to create plain text results from your HTML like mails, with slightly better results in first library but at cost of XML dependency. Using these libraries will allow to use real HTML, for more compatibility with different clients...

slavkoja commented 3 years ago

I am able to get desired output with this:

<?php
/** @var string $subject */
/** @var \Illuminate\Support\Collection $items */
/** @var \Fisharebest\Webtrees\Fact $fact */

echo "<h2>$subject</h2>\n\n";

foreach ($items as $group => $facts) :
    echo "<h3>$group</h3>\n";
    echo "<ul>\n";
    foreach ($facts as $fact) :
        $record = $fact->record();
        echo "  <li>" . $fact->label() . ': ';
        echo '<a href="' . e($record->url()) . '" style="text-decoration:none;">' . strip_tags($record->fullName()) . "</a> ";

        $year = $fact->date()->display(false, '%Y');
        if (!empty($year))
            echo "(" . strip_tags($year) . " / " . strip_tags($fact->anniv) . ")";
        echo "</li>\n";
    endforeach;
    echo "</ul><hr />\n";
endforeach;

// signature
echo '<p style="color:#444;">-- <br />'."\n";
echo "webtres</p>";

Notice, i changed the HTML slightly, i use unordered list of group's facts (ul), h2/h3 tags for titles and some styles are added. With this i got plain text:

4 apríla

  Narodenie: Name Hidden (1962 / 59)

At end i add signature placeholder. IMO, here have to be info about unsubscribe, or at least link to webtrees, but i do not know how to get this URL.

I will suggest to add List-ID and List-Unsibscribe headers too, but i will create separate issue for this latter.

UksusoFF commented 3 years ago

List-ID and List-Unsibscribe is webtrees issue. Modules can't attach this headers.

slavkoja commented 3 years ago

Not really ;-) only its send method is not suitable for purpose of this module. Because its transport method (thus mail settings) is reusable, it seems to be simple to implement...

I will try it latter as i recently did little play with SwitMailer, but my PHP knowledge are (very) limited (e.g. no OOP), it will need to be polished...

UksusoFF commented 3 years ago

@slavkoja try latest master version. Seems plain text message looks fine now.

slavkoja commented 3 years ago

I will try soon.

But are you sure, that it is better to fix ugly HTML by calling four functions (including expensive regexp) instead of fixing HTML and stay on one function call?

UksusoFF commented 3 years ago

No ;)

slavkoja commented 3 years ago

Do you want to help with HTML?

UksusoFF commented 3 years ago

You can use own html if you wish. For me prefer more readable variant without closing tags far away from opened :)