-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Domains: Implement multi-target email forwards #98837
base: trunk
Are you sure you want to change the base?
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~1603 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~22828 bytes removed 📉 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~16733 bytes added 📈 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/17215663 Some locales (Hebrew, Japanese, Spanish) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday. Hi @alshakero, could you please edit the description of this PR and add a screenshot for our translators? Ideally it'd include all of the following strings:
Thank you in advance! |
@crisbusquets Hi there! I built what I thought made sense while design is being done. Would be great if you can review what I came up with 🙏🏼 You can use the Live link, then go to /domains/manage/all/email/alshaker.me/alshaker.me (I added you to the site) and look at it. Sadly you won't be able to make changes unless you do some sandboxing and such. |
|
||
/** | ||
* @param newEmailForward a string representing a new email forward | ||
* @returns { boolean } If the email forward is has more than the maximum number of destinations. |
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.
Probably need to rephrase this
@alshakero - I was testing the validation on multiple fields simultaneously and noticed this behavior where the Add button isn't enable. I don't think the average person will do this, but I just wanted to point it out. Also, erasing an email removes all the valid and invalid emails below it. CleanShot.2025-01-28.at.18.50.34.mp4 |
In the code I see that it is a current limitation that we have, worth it to double check probably. I'm sure @alshakero already did that. |
Hi @crisbusquets @auareyou I implemented the form. Working on the list later tonight. Please take it for a drive at /overview/site-domain/email/alshaker.me/forwarding/add/alshaker.me |
This reverts commit 5a22366.
</Heading> | ||
<Text> | ||
{ translate( | ||
"This will remove it from our records and if it's not used in another forward, it will require reverficiaton if added again." |
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.
"This will remove it from our records and if it's not used in another forward, it will require reverficiaton if added again." | |
"This will remove it from our records and if it's not used in another forward, it will require reverification if added again." |
import { Mailbox } from '../../../../data/emails/types'; | ||
import './style.scss'; | ||
|
||
export function VerificatonPendingNotice( { warnings }: { warnings: Mailbox[ 'warnings' ] } ) { |
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.
Spelling: Lets update VerificatonPendingNotice
to VerificationPendingNotice
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
Requires https://github.a8c.com/Automattic/wpcom/pull/171065
Related to #98801
Proposed Changes
This implements the ability to add multiple email forwards for a single mailbox. It allows up to 5 addresses per mailbox.
Why are these changes being made?
See: p58i-jdB-p2
Testing Instructions
Testing under Upgrades > Emails
Validation
Deletion
Verification
Testing under /sites > Domains > Email (Tab)
Repeat all the above. The only difference, is that "Add mailbox" is on top now.
Testing under Hosting settings
Repeat all the above. The only difference, is that "Add mailbox" is on top now.
URL: /overview/site-domain/email/DOMAIN/DOMAIN
Regression testing (important)