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

Purchases: Adjust modal message when deleting a Domain Mapping subscription. #55399

Merged
merged 7 commits into from
Sep 3, 2021

Conversation

chrisfromthelc
Copy link
Contributor

@chrisfromthelc chrisfromthelc commented Aug 11, 2021

Changes proposed in this Pull Request

Testing instructions

  1. Add a domain mapping to any plan.
  2. Navigate through Upgrades > Domains > click on the domain mapping.
  3. Click Delete Domain Mapping at the bottom.

You should be presented with a modal showing an updated message:

Are you sure you want to remove DOMAIN from SITEADDRESS? You will not be able to reuse DOMAIN again without starting a new Domain Mapping subscription.

Screen Shot 2021-08-11 at 3 38 15 PM

Related to #49043

@chrisfromthelc chrisfromthelc added [Type] Bug [Feature Group] Emails & Domains Features related to email integrations and domain management. User Report This issue was created following a WordPress customer report labels Aug 11, 2021
@chrisfromthelc chrisfromthelc self-assigned this Aug 11, 2021
@github-actions
Copy link

github-actions bot commented Aug 11, 2021

Link to live branch is being generated...
Please wait a few minutes and refresh this page.

@matticbot
Copy link
Contributor

matticbot commented Aug 11, 2021

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~104 bytes added 📈 [gzipped])

name            parsed_size           gzip_size
site-purchases       +879 B  (+0.1%)     +104 B  (+0.0%)
purchases            +879 B  (+0.1%)     +104 B  (+0.0%)
domains              +879 B  (+0.1%)     +104 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

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.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Contributor

@DavidRothstein DavidRothstein left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

client/me/purchases/remove-purchase/index.jsx Outdated Show resolved Hide resolved
@chrisfromthelc
Copy link
Contributor Author

chrisfromthelc commented Aug 13, 2021

@DavidRothstein After thinking on this further and digging around a bit more this evening, I'm wondering if it might be advantageous to treat removing a mapping like we would a registered domain. The registered domain code handling the deletion messaging has a series of checks to see if the user's account email is based on the deleting domain, if there is Titan email for the domain, etc.

https://github.com/Automattic/wp-calypso/blob/trunk/client/me/purchases/remove-purchase/remove-domain-dialog/index.jsx

I have encountered a number of users who have deleted/let expire a domain mapping where there were secondary effects like these, so it seems prudent to take similar steps like we would with a registered domain. We can have a customized modal/flow for mapping and rework the other text (what was originally to be changed with this PR) to something that works better for the other products.

@DavidRothstein
Copy link
Contributor

@chrisfromthelc - hm, interesting idea! I guess it's true that removing a domain mapping is similarly destructive to a site as removing a domain registration. (Although the main difference is that removing a domain mapping is easier to recover from if you do it by mistake.)

In principal I do think something like this could work and would make sense, but I wonder how much effort is involved. It seems to me like it would probably be a lot harder than just changing the text of the existing dialog... So overall, I don't know 🤷

@chrisfromthelc
Copy link
Contributor Author

In principal I do think something like this could work and would make sense, but I wonder how much effort is involved.

Since this isn't a very urgent issue, I'm looking into this to see what I can figure out, and it'll be a good learning experience on building out this kind of functionality.

@chrisfromthelc chrisfromthelc marked this pull request as ready for review August 30, 2021 15:52
@chrisfromthelc chrisfromthelc requested a review from a team as a code owner August 30, 2021 15:52
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 30, 2021
@chrisfromthelc
Copy link
Contributor Author

I reworked this, adding a new render for Domain Mapping specifically:

Screen Shot 2021-08-30 at 3 31 06 PM

The plans modal will have the original version of the message:

Screen Shot 2021-08-30 at 3 30 43 PM

We can tweak the messaging further if needed, but now that they're completely separated, we don't need to genericize the text as much as originally planned.

@chrisfromthelc chrisfromthelc linked an issue Aug 30, 2021 that may be closed by this pull request
client/lib/purchases/index.js Outdated Show resolved Hide resolved
@chrisfromthelc chrisfromthelc force-pushed the update/domain-mapping-delete-modal branch from e8f48a2 to 4eda3f9 Compare September 1, 2021 00:02
client/me/purchases/remove-purchase/index.jsx Outdated Show resolved Hide resolved
client/me/purchases/remove-purchase/index.jsx Outdated Show resolved Hide resolved
@michaeldcain michaeldcain removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 2, 2021
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 2, 2021
@jjchrisdiehl
Copy link
Contributor

Tested - looks good to me:

Domain mapping

image

Plans

image

@chrisfromthelc chrisfromthelc merged commit 36fe49f into trunk Sep 3, 2021
@chrisfromthelc chrisfromthelc deleted the update/domain-mapping-delete-modal branch September 3, 2021 18:46
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 3, 2021
@a8ci18n
Copy link

a8ci18n commented Sep 3, 2021

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/6546680

Thank you @chrisfromthelc for including a screenshot in the description! This is really helpful for our translators.

@a8ci18n
Copy link

a8ci18n commented Sep 20, 2021

Translation for this Pull Request has now been finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature Group] Emails & Domains Features related to email integrations and domain management. [Type] Bug User Report This issue was created following a WordPress customer report
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mappings: Confusing Message When Removing It
6 participants