Remove the warning about the Umami profile on the install screen

Based on the parent issue, I think this is supposed to be about all the install screen wording (including the profile description too), not just the warning message?

In any case, here's a possible patch. See #2938189-9: [META] Finalise the wording of the messages in installer, toolbar and on the status report page for discussion and screenshots.

Thanks @markconroy and @David_Rothstein!

+++ b/core/profiles/demo_umami/demo_umami.info.yml
@@ -1,6 +1,6 @@
+description: "Install an example website that shows off Drupal's capabilities.<br><div role='contentinfo' aria-label='Warning message' class='messages messages--warning'>This creates a site which is intended for demonstration purposes.</div>"

Discussed in the UX meeting, we agreed on changing the warning text to "This creates a site that is intended..."

For the rest, people in the call said like the change was ok.

Anyway, I agree with @markconroy about not promising the moon. So +1 to "shows off some of Drupal's capabilities.".

But it was pointed out that the < strong > tag used to stand out the "Experimental" word appears in the Status page too and it shouldn't.

I think we need a qualificiation within the text, so that it doesn't promise everything - "Install an example website that shows off Drupal's capabilities.".

Maybe something like:
"Install an example website that shows off some of Drupal's capabilities."

Yeah, adding "some of" back in sounds good to me - I may have removed too many words there :)

Discussed in the UX meeting, we agreed on changing the warning text to "This creates a site that is intended..."

It's a very minor point, but I slightly prefer "which" over "that" because it sounds a little better when you read it; otherwise you can get tongue-tied with the "this" and "that". (Some people think "that" is the only grammatically-correct choice for a sentence like this, but that is not actually the case. See https://www.merriam-webster.com/words-at-play/when-to-use-that-and-which.) But I think either would be fine.

But it was pointed out that the < strong > tag used to stand out the "Experimental" word appears in the Status page too and it shouldn't.

Yes, probably needs to be fixed in #2822414: Redesign the 'install profile selection' installer screen to allow for experimental profiles and more information (to allow the experimental profile to be labeled on the install page without including it as part of the profile name). Or alternatively, don't display that word as part of the title at all, which I guess is one of the designs that has been proposed there also.

I'm not a native language speaker, so I'll trust you all. :) (It was mentioned by It was suggested by @benjifisher in the UX meeting)

About the tag in the title, +1 to @David_Rothstein's suggestion to remove it since it is not in the final design chosen for the MVP.

OK, here's a new patch that adds "some of" (and I also changed "website" to "site" because I noticed that "site" is what is used in the Minimal profile description, so this should probably be consistent with that.)

And, a second patch that additionally changes "which" to "that". That way someone else can decide between them :) (Actually, I am now wondering if it is even better to just use neither word - could it simply be "This creates a site intended for demonstration purposes" instead?)

It sounds like there is still discussion ongoing about the profile title. Both "Food Magazine" and "Experimental" are important pieces of information, so if one or both of those is removed from the title, wouldn't it have to be added to the text somewhere else (e.g. the description)?

fixed as per #15

One slight concern I have with moving from this warning text (which my patch above had):

This creates a site that is intended for demonstration purposes.

To this (from #15):

This profile is intended for demonstration purposes only.

is that I wonder if the second gives people the impression that this isn't a "real" profile (i.e., that the option on the screen is itself just there for demo purposes, and that clicking on the radio button won't actually do anything)?

I do think they'll figure it out eventually, but I would not be surprised if some people hesitate briefly out of confusion -so that is why I went with the "creates a site" text originally.

This issue's suggestion are going to be addressed in #2938185: When installing Umami, only show warning if 'Demo Umami' radio button is selected (and ensure that it is obvious that warning message only applies to the Umami profile). Marking as postponed until that one is closed, and the probably we'll be able to close this one.

I'll move @David_Rothstein suggestion to the other issue so it doesn't get lost.

Blocker is in

With that issue committed, I think the one remaining thing here is to discuss my idea from #19.

Here is a patch for that, with "This creates a site intended for demonstration purposes" as the final wording of the warning. (Note that this also makes things a bit more consistent with the proposed wording in #2938800: Finalize wording for Toolbar warning message.)

However, since most of the wording was finalized in the other issue (and this is just a small tweak) I think this can definitely be downgraded and shouldn't block a beta release or anything like that.

I am agreed to the wordings suggested by @David_Rothstein, I am leaving this to 'Needs Review' to be reviewed by others too, but RTBC ;-)

I am a bit mixed about this change. Two thoughts:

1. "This" looks odd. The warning only shows when you have the profile selected, so what else would the warning be about? We don't have "This" for field descriptions or anything similar because it is evident what the description is about (not that element but this element :).
2. With the removal of "only" the *warning* message looks like an *informational* message to me now.

So I would at least remove "This" => "Creates a site intended for demonstration purposes". The "only" vs. warning probably needs a bit of discussion.

I see what you're saying, but whereas we can get away with incomplete sentences for a field description sometimes, I don't think we can for a standalone message. It should be a complete sentence.

So if the dangling "this" looks wrong, I guess another option would be "This profile creates a site intended for demonstration purposes". It's a little longer, but it's consistent with other warnings (e.g. "This action cannot be undone" for Drupal's standard confirmation message).

With the removal of "only" the *warning* message looks like an *informational* message to me now.

Any reason you're worried about it here but not in #2938800: Finalize wording for Toolbar warning message? Removing "only" here just matches what was done there.

That said, you are not wrong about that making it a bit more of an informational message - personally, that's where I'd like to see it go anyway (#2945378: Finalize design for Toolbar warning message, and change the install screen design to match) :)

In my mind the lack of "this" does not make the sentence incomplete. Neither of the other text use "this" on this screen, because it is always evident which option they are about. It is not "Save and continue this installation" or "Pick this option to install with commonly used features pre-configured."

I don't know why it feels like removing the "only" word here is not ok if it was in #2938800: Finalize wording for Toolbar warning message. It may be that the visual of it there is non-standard otherwise so it did not feel like needing to satisfy "being a warning". But if we want to make it an informational message, I don't think we should keep it a warning wrapper, because it does not line up conceptually.

Closing as addressed by #2938185

The patch here still applies (with some offset). That other one is a different issue.

@David_Rothstein the last submitted ptach-23 applied cleanly , you can see the screenshot after applying patch i can see the changes in wording about the Ummai Profile so there is no ned to rework it again

My mistake @David_Rothstein - apologies.

It would be super helpful to update the issue to reflect the current status and rationale behind this change, maybe with new screenshots as those in the summary are totally outdated.

working on at #DrupalEurope

Unassigning PunamShelke. Please respond if you are still working on this issue.

Because many people are working on/looking for issues marked Novice and DrupalEurope today at the Mentored Sprint, we ask people to not assign these issues to themselves today. But instead write a comment that they are started working and (if applicable) stopped working without a result.

It seems right after applying patch #23.
Screenshot before:

Screenshot after applying patch:

@lauriii asked me to review this again. I stand by my prior comments on the change:

- "This" looks superfluous, it is evidently about this option, we don't put "this" into any other description or field label
- Since the new message is more informational than a warning (in no small part due to the removal of "only" at the end), it now looks odd in the warning wrapper. Why is it a problem/issue that the site is for demonstration purposes? That is not explained. Then why display it as a problem?

I am not convinced we need the warning label at all. We already have "Demo" in the radio button and "example site" in the description, I don't think we need to re-iterate that for a third time with a scary-looking warning triangle.

The profile selection page without the warning on Umami :

And the patch.

Due to the removed warning, one test failed:

Drupal\FunctionalJavascriptTests\Core\Installer\Form\SelectProfileFormTest::testUmamiProfileWarningMessage
Error: Call to a member function isVisible() on null

/var/www/html/core/tests/Drupal/FunctionalJavascriptTests/Core/Installer/Form/SelectProfileFormTest.php:127

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

I removed the test case completely as (in my opinion) it does not make sense to test the demo profile with a special case compared to the other options without the warning. No visible changes were made compared to patch and comment #45.

I've readded the test that I removed in comment #48 and so tried to fix the test that failed in comment #50. Please review.

Tried to fix the test that failed in comment #51. Please review.

Looks good.

We should update the issue summary with the proposed solution, and after that get a product manager to review it.

Tagging for upcoming contribution days.

The last patch failed to apply on 9.2, so re-rolled for 9.2

Fixed test, Please have a look.

The warning message is hidden now. Adding before and after patch screenshot for reference.

Updated the IS to match the recent discussion and with screenshots that match the latest patch.

Also removing "Novice" as there is nothing left for a novice to do here.

Marking RTBC for product manager review.

Actually, the updated test could do with a little improvement:

+++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/Installer/Form/SelectProfileFormTest.php
@@ -119,19 +119,27 @@ protected function refreshVariables() {
+    $page->selectFieldOption('profile', 'standard');
+    $profile = $page->find('css', 'label[for="edit-profile-standard"]')->getText();
+    $this->assertTrue(($profile == 'Standard'));

This would be simpler as just

$this->assertSession()->optionExists('profile', 'Standard');

repeated for each profile option.

@longwave I have worked on the above suggestion mentioned in #67, however its not working and gives the below error message.
"Select with id|name|label|value "profile" not found."

According to me its not select field, Please advise.

Ah yeah that only works for select, maybe we can just do

    $this->assertSession()->fieldExists('Standard');
    $this->assertSession()->fieldExists('Minimal');
    $this->assertSession()->fieldExists('Demo: Umami Food Magazine (Experimental)');

?

Not even sure we need the test, it was added in #2938185: When installing Umami, only show warning if 'Demo Umami' radio button is selected (and ensure that it is obvious that warning message only applies to the Umami profile) when we added the warning, now we are removing the warning maybe we should just delete the test.

In fact we already have multiple tests that exercise each of these options, anything that descends from InstallerTestBase will use one of the radio buttons to install the profile so this test can just be removed - we don't need to duplicate it. It was added separately here because of the JavaScript interaction with the warning, but now that has been removed we can also remove the JavaScript test.

Verified and tested updated patch #53 and patch #64
applied successfully.

@markconroy As mentioned in comment #71, Removed the JavaScript test, Patch given for 9.2.x, Please have a look and advise.

Thanks!

I don't know if people who felt strong before about the warning changed their minds or not. I don't think that is needed personally. You would fall into the "building off of Umami trap" after you installed it, not before. And the runtime still has the warning. So I think making trying Umami more welcoming is better. We don't want to scare people off trying Umami, just scare them off of building on top of it. And that is at a later stage.

Discussed with @catch to make sure the release managers don't have concerns on this change.

Committed 62e06d8 and pushed to 9.3.x and 9.2.x. Thanks!