-
Notifications
You must be signed in to change notification settings - Fork 9
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
P#63827 Add a default email address #186
Conversation
#139 was merged today 🎉 |
…s' into 63827_add_a_default_email_address # Conflicts: # languages/onoffice-for-wp-websites-de_DE.mo # languages/onoffice-for-wp-websites-de_DE.po # languages/onoffice-for-wp-websites-es_ES.mo # languages/onoffice-for-wp-websites-es_ES.po # languages/onoffice-for-wp-websites-it_IT.mo # languages/onoffice-for-wp-websites-it_IT.po # languages/onoffice-for-wp-websites-nl_NL.mo # languages/onoffice-for-wp-websites-nl_NL.po # languages/onoffice-for-wp-websites.pot # plugin/Controller/AdminViewController.php # plugin/Installer/DatabaseChanges.php
Pull Request Test Coverage Report for Build 2407625054
💛 - Coveralls |
update getCountDefaultRecipientRecord func add param to contruct of displayUsingEmptyDefaultEmailError func
add escaping to recipient email
…_email_address # Conflicts: # languages/onoffice-for-wp-websites-de_DE.mo # languages/onoffice-for-wp-websites-es_ES.mo # languages/onoffice-for-wp-websites-it_IT.mo # languages/onoffice-for-wp-websites-nl_NL.mo
Steps to install the approved version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Steps to install the approved version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- For the contact form, the email override starts out deactivated as desired. For interest and owner forms, it starts out activated and when the default email is active, it suddenly gets disabled after loading. Please make it consistent with the contact form for all form types.
- In the contact form, the label for the email is "Override email address", but for the other form types it is "Override Recipient's E-Mail Address". Please make it consistent and use "Override email address" for all form types.
Steps to install the approved version:
|
@jmaas-onoffice On master branch, email input in all forms has type 'text', so I update it to 'email' type and add email validation when saving form. |
@LongTrong-exe Yes, that sounds good, thanks. I think we had some discussion on how to validate email addresses in another PR, but I can't find it. The conclusion was: Don't do anything complicated, |
Oh, I didn't see that you already requested my review. I will have a look! |
js/ajax_settings.js
Outdated
var typesNeedValid = ['email']; | ||
|
||
form.submit(function (e) { | ||
e.preventDefault(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused by this. Why do we need to prevent the form from sending and validate and save it ourselves here? Shouldn't that be something we do in #160?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to fix that in #160
This reverts commit 06f20a6
# Conflicts: # plugin/Installer/DatabaseChanges.php
This reverts commit b44899a.
Steps to install the approved version:
|
No description provided.