Skip to content

Conversation

@php-schubser
Copy link
Contributor

No description provided.

Copy link
Owner

@RobinTheHood RobinTheHood left a comment

Choose a reason for hiding this comment

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

Looks very well. Thank you. I have some notes for you. 😊 Maybe you can fix that and make a commit.

$headers[] = 'From: '.$from.' <'.$fromEmail.'>';
if (mail($to, $subject, $message, implode("\r\n", $headers))) {
Notification::pushFlashMessage(
array(
Copy link
Owner

Choose a reason for hiding this comment

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

Looks nice, but can you use short version of Array. Only [...]

$message = ArrayHelper::getIfSet($_POST, 'message', '');
if ($fromEmail == '' || $from == '' || $message == '') {
Notification::pushFlashMessage(
array(
Copy link
Owner

Choose a reason for hiding this comment

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

Looks nice, but can you use short version of Array. Only [...]

if ($fromEmail == '' || $from == '' || $message == '') {
Notification::pushFlashMessage(
array(
"text" => "Warnung: Felder k&ouml;nnen nicht leer gelassen werden.",
Copy link
Owner

Choose a reason for hiding this comment

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

You can use ö because we use UTF8. 😊

$message .= 'PHP version: ' . phpversion() . '<br />';
$headers[] = 'MIME-Version: 1.0';
$headers[] = 'Content-type: text/html; charset=iso-8859-1';
$headers[] = 'From: '.$from.' <'.$fromEmail.'>';
Copy link
Owner

Choose a reason for hiding this comment

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

Can you put a space before and after the . (Dot) operator 😊

fix: changed array to short version
fix: added space before / after dot operator
Copy link
Owner

@RobinTheHood RobinTheHood left a comment

Choose a reason for hiding this comment

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

I like your changes. Thank you. I have found only one little note. 😊

<?php
if (isset($_POST['send_mail'])) {
SendMail::sendFeedback();
} ?>
Copy link
Owner

Choose a reason for hiding this comment

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

Can you put logical code (line 3-8) in the your new Methode invokeReportProblem () in IndexController. I know that the IndexController has to much code. In the future we can split up the IndexController in Multiple Controllers. 😊

@RobinTheHood
Copy link
Owner

RobinTheHood commented Aug 11, 2020

Maybe we can add the Domain to the E-Mail. 🤓🤔❓😊

fix: move logical code from template to controller
fix: changed charset for mail
@php-schubser
Copy link
Contributor Author

I'm finished with changes, I hope you like it 😄

Copy link
Owner

@RobinTheHood RobinTheHood left a comment

Choose a reason for hiding this comment

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

Looks very nice. Thank you very much. I like it. 👌🏻🤓🎉

@RobinTheHood RobinTheHood merged commit 9251039 into RobinTheHood:master Aug 11, 2020
@RobinTheHood RobinTheHood added the enhancement New feature or request label Aug 18, 2020
@RobinTheHood RobinTheHood added this to the First Public Beta milestone Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants