Migrate UI: Initial Screen UX improvements

Just clarifying things a bit. Thanks for reviewing and starting the issue.

Tagging

This is a blocker for #2905491: Mark Migrate Drupal UI as stable

1. I tried to find *parts* for each bullit to emphasize but some would need the whole sentence to be bolded so maybe don't emphasize anything?
2. I incorporated the one-shot deal thing into the first bullit, does that work?
3. Added a old site/new site explainer to make the actual bullits a bit easier to read.

---

Upgrade a site by importing its database and files into a clean and empty new install of Drupal 8. See the [Drupal site upgrades handbook] for more information.

• Old site: the site you want to upgrade
• New site: this empty Drupal 8 installation you will import the old site into.

1. You may need multiple tries for a succesful upgrade so backup the database for this new site. The upgrade will change it and you may want to revert to its initial state.
2. Make sure that the database for the old site is accessible on the host of this new site.
3. If the old site has private files, a copy of its files directory must also be accessible on the host of this new site.
4. Enable all modules on this new site that are enabled on the old site. For example, if the old site uses the book module, then enable the book module on this new site so that the existing data can be imported into it.
5. Do not add any content to this site before upgrading. Any existing content is likely to be overwritten by the upgrade process. See [the upgrade preparation guide].
6. Put this site into [maintenance mode].

The upgrade can take a long time. It is better to upgrade from a local copy of your site instead of directly from your live site.

I quite like the old site/new site definitions.

One question is the about the phrases 'import the old site into' and 'imported into it'. Is that right? I mean I would just say, 'import the old site to' and 'imported to it'. I don't mind at all what is used, I am just curious.

Thanks! Me no native speaker so by all means correct or simplify where necessary :)

And I am old enough that my native language has been changed by the young ones (as they should). I am not sure what is correct, either!

from #5 and #6.

1. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,25 @@ class MigrateUpgradeForm extends ConfirmFormBase {
   +      $legend[] = $this->t('<em>Old site:</em>  the site you want to upgrade');
   +      $legend[] = $this->t('<em>New site:</em> this empty Drupal 8 installation you will import the old site to.');
   

   Let's add a title/label of definitions to these.

2. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,25 @@ class MigrateUpgradeForm extends ConfirmFormBase {
   +      $info[] = $this->t('You may need multiple tries for a succesful upgrade so backup the database for this new site. The upgrade will change it and you may want to revert to its initial state.');
   

   Let's bold 'backup the database'

3. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,25 @@ class MigrateUpgradeForm extends ConfirmFormBase {
   +      $info[] = $this->t('Make sure that the database for the old site is accessible on the host of this new site.');
   

   Let's change that to "Make sure that <strong>access to the database</strong> for the old site is available from this new site."

4. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,25 @@ class MigrateUpgradeForm extends ConfirmFormBase {
   +      $info[] = $this->t('If the old site has private files, a copy of its files directory must also be accessible on the host of this new site.');
   

   Bold, "If the old site has private files"

5. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,25 @@ class MigrateUpgradeForm extends ConfirmFormBase {
   +      $info[] = $this->t('Enable all modules on this new site that are enabled on the old site. For example, if the old site uses the book module, then enable the book module on this new site so that the existing data can be imported to it.');
   

   Bold, "Enable all modules on this new site"

6. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,25 @@ class MigrateUpgradeForm extends ConfirmFormBase {
   +      $info[] = $this->t('Do not add any content to this site before upgrading. Any existing content is likely to be overwritten by the upgrade process. See <a href=":url">the upgrade preparation guide</a>.', [
   

   Bold: "Do not add any content to the new site".
   And let's use the term from our definition and call it new site.

Agreed with all those changes @heddn, you found the right parts to make bold after all :)

+++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
@@ -142,16 +142,25 @@ class MigrateUpgradeForm extends ConfirmFormBase {
+      $legend[] = $this->t('<em>New site:</em> this empty Drupal 8 installation you will import the old site to.');

I just now reviewed #6 a little more closely. Isn't there something we were taught about dangling prepositions? I seem to recall something about that. How's this to improve things?

<em>New site:</em> this empty Drupal 8 installation into which you will import the old site.

working in this.

@heddn, that is one of the myths that you and I were taught.

https://newrepublic.com/article/113187/grumpy-grammarian-dangling-prepos...
And more here, https://en.wikipedia.org/wiki/English_usage_controversies

1. I don't know what label to use.

All others point fixed.

The bolding looks good to me. Reading down, every line has some emphasis, and it even built up the expectation that each line would. And then point 6 has none, and I was surprised. But I guess it makes sense since the link is the important bit.

Reviewed in weekly migrate meeting and agreed this is in fact a migrate critical.

Updated screenshot with an attempt at adding a label to these things.

Fixing tests.

Looking good. Just one reservation.

+++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
@@ -142,16 +142,26 @@ public function buildOverviewForm(array $form, FormStateInterface $form_state) {
+      $legend[] = $this->t('<em>New site:</em> this empty Drupal 8 installation you will import the old site.');

'you will import the old site.' To where? Still go the for preposition at the end 'to'. So it becomes, 'you will import the old site to.' Far better than the odd construction 'into which you will import the old site.'

Maybe we need some else to offer their opinion?

This is what I am thinking. To keep RTBC, I'll resist making the change myself.

+++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
@@ -142,16 +142,26 @@ public function buildOverviewForm(array $form, FormStateInterface $form_state) {
+      $legend[] = $this->t('<em>New site:</em> this empty Drupal 8 installation you will import the old site.');

s/site/site to./

Thanks!

With such a small change, I'll RTBC this while I am here.

1. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,26 @@ public function buildOverviewForm(array $form, FormStateInterface $form_state) {
   +      $legend[] = $this->t('<em>Old site:</em>  the site you want to upgrade');
   

   nit: there are two spaces here - can be fixed on commit

2. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,26 @@ public function buildOverviewForm(array $form, FormStateInterface $form_state) {
   +      $info[] = $this->t('You may need multiple tries for a succesful upgrade so <strong>backup the database</strong> for this new site. The upgrade will change it and you may want to revert to its initial state.');
   

   nit: successful (two s's) - same - can be fixed on commit

1. Another nit: one of the definitions ends with a period. The other does not
2. "Important notes" is almost meaningless. Everything on a screen is important or we wouldn't show it :) Can we do something like "Steps to prepare for the upgrade." or "Take these steps before upgrading" or "How to prepare for the upgrade"?

Corrected nits highlighted in #31 and #32.

Corrections don't make any functional difference so returning to RTBC.

Agreed. Thanks for the quick turnaround @Jo Fitzgerald!

Committed as 6cbb549 and pushed to 8.5.x

Cherry-picked as 9ffe00f and pushed to 8.4.x

Nice work on this. A few points:

1. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,26 @@ public function buildOverviewForm(array $form, FormStateInterface $form_state) {
   -        '#markup' => '<p>' . $this->t('Upgrade a site by importing it into a clean and empty new install of Drupal 8. You will lose any existing configuration once you import your site into it. See the <a href=":url">online documentation for Drupal site upgrades</a> for more detailed information.', [
   +        '#markup' => '<p>' . $this->t('Upgrade a site by importing its database and files into a clean and empty new install of Drupal 8. See the <a href=":url">Drupal site upgrades handbook</a> for more information.', [
   

   We're not importing the site's database and files. We're importing the data from its database and files.

2. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,26 @@ public function buildOverviewForm(array $form, FormStateInterface $form_state) {
   +      $legend[] = $this->t('<em>Old site:</em> the site you want to upgrade.');
   +      $legend[] = $this->t('<em>New site:</em> this empty Drupal 8 installation you will import the old site to.');
   +
   +      $form['legend'] = [
   +        '#theme' => 'item_list',
   +        '#title' => $this->t('Definitions'),
   +        '#list_type' => 'ul',
   +        '#items' => $legend,
   +      ];
   

   If it's a list of definitions, should it use a definition list?

3. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -142,16 +142,26 @@ public function buildOverviewForm(array $form, FormStateInterface $form_state) {
   +      $info[] = $this->t('You may need multiple tries for a successful upgrade so <strong>backup the database</strong> for this new site. The upgrade will change it and you may want to revert to its initial state.');
   

   "Back up" needs to be two words when it's a verb. (It's correct in HEAD.)

   Also "you may need multiple tries" is kind of confusing ("a try" as a noun might not be a familiar expression); "attempts" might be better. But reading this I might wonder why I might need multiple attempts; it's not like shooting a basketball.

   Finally, we aren't using the defined term "new site" here, and if I'm starting from a "clean and empty new install" then why would I want to back up the database of that?

4. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
   @@ -160,12 +170,13 @@ public function buildOverviewForm(array $form, FormStateInterface $form_state) {
   +        '#title' => $this->t('Steps to prepare for the upgrade'),
   

   How about "Preparation steps"?

Sorry, I totally opened this tab this morning and didn't see that @larowlan committed it hours ago before I reviewed. Things in #39 can be a followup if preferred.

Created a follow-up to address @xjm's comments: #2908699: Several textual and markup improvements to the Migrate Upgrade UI