Install Messages

Hm, I'm not a fan of install messages. People can already find a link to the config page on the module list (see image). What would be a good idea though is explaining on the feed type listing page, that you first need to create a feed type and then a feed to import content.

Okay, I agree that it can be helpful for new users to know where to go next after enabling the Feeds module. And I agree that all the help texts provided by the Webform module can be a good help for new users.

I do think that the message "Thanks for installing Feeds" is not needed, because the message above already says that Feeds got enabled. The Webform module provides links to their online documentation:

That would probably a good idea to do here too. I think it should link to the following page:
https://www.drupal.org/docs/8/modules/feeds/creating-and-editing-import-...
Personally, I would omit the "Congratulations" message. To me that sounds like as if I just completed a hard task, while I only enabled a module.

A link to the feed type listing page like you already did in the patch from #2 could a be a good idea too to add to the install message. Maybe the message should be something like this "Start with adding a feed type".

As said in #3, I think the feed type listing page should mention that first you need to create a feed type and then - to actually import content - create a feed of that type.

The message for the feed type listing page should be implemented by implementing the hook hook_help(). For the install message, it would be nice if we can implement that in a similar as Webform does, if that is feasible (I mean: only do this if we don't need tons of code to achieve that).

For Webform I meant both the visual and functional part.

Thinking about it again, maybe it's enough for Feeds to only display the message once, to keep it simple. The Webform install message stays on the module page until you click the cross.

Okay, forget about the Webform approach then. Just adjust the messages from the patch in #2 so that incorporates my other suggestions from #6.

Thanks for the patch! Here is my review.

1. +++ b/feeds.install
   @@ -12,6 +14,20 @@ function feeds_uninstall() {
   +  $messenger->addStatus(
   +    t('Module <em class="placeholder">Feeds</em> has been enabled.')
   +  );
   

   We don't need these lines. Drupal says by default which modules got enabled.

2. +++ b/feeds.install
   @@ -12,6 +14,20 @@ function feeds_uninstall() {
   +  $messenger->addStatus(t('Start with adding a <a href="@url_settings">feed type</a> or access the <a target="feeds-guide" href="https://www.drupal.org/docs/8/modules/feeds/creating-and-editing-import-feeds">guide</a>.', [
   

   The link to the guide should be in a placeholder, just as you did for the other link (which placeholder is @url_settings).

   The reasons for why an url in a translatable string should always be a variable are the following:

   • Translators cannot mistype the url in the translation.
   • If the url would change in the future, the translatable string would stay the same, so it wouldn't need to be retranslated.

Another remark: I think it's better to name the placeholder for add feed type link @feed_type_add_url instead of @url_settings, because that better describes the url that is going to be displayed.

+++ b/feeds.install
@@ -12,6 +14,18 @@ function feeds_uninstall() {
+  $messenger->addStatus(t('Start with adding a <a href="@feed_type_add_url">feed type</a> or access the <a target="feeds-guide" href="@guide_url">guide</a>.', [
+    '@feed_type_add_url' => Url::fromRoute('entity.feeds_feed_type.add_form')->toString(),
+    '@guide_url' => 'https://www.drupal.org/docs/8/modules/feeds/creating-and-editing-import-feeds',
+  ]));

Whilst I agree with @MegaChriz that we don't need to say congratulations or tell the user that the module has been installed, if the module is installed with a batch of others that generate messages, there is no way currently to know that this message is about Feeds.

Suggest full message text be (links abbreviated for readability here):

To configure feeds, start by adding a <a>feed type</a>. For more information <a>read the guide</a>.

Updating the patch based on #18. Please review.

Thank you

Thanks @srilakshmier :)

Normally I would set this NW and tag it 'needs tests' but I don't know how this module likes that handled and I notice a maintainer has been in here and not mentioned it, so I'll leave it to the maintainers to decide if this needs a test.

RTBC

Thanks for contributing. Committed #19 with one small change: for links I found out that the placeholder : (instead of @) is recommended in D8/D9.

See https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Rend...

As with @variable, do not use this within HTML attributes, JavaScript, or CSS. Doing so is a security risk.
:variable: Return value is escaped with \Drupal\Component\Utility\Html::escape() and filtered for dangerous protocols using UrlHelper::stripDangerousProtocols(). Use this when using the "href" attribute, ensuring the attribute value is always wrapped in quotes

@darvanen
Maybe not every little thing needs a test, so I committed it without one this time. Thanks for reviewing!

FYI when installed from command line:

composer require 'drupal/feeds:^3.0@alpha'
...
  - Downloading drupal/feeds (3.0.0-alpha11)
...

Everything looks ok.
But now I'm going to install this module:

drush -y en feeds

In RouteProvider.php line 206:

  Route "entity.feeds_feed_type.add_form" does not exist.

Same will happen again if I uninstall module (drush -y pmu feeds) and install it again.
Tested with Drupal version 9.2.10

Thanks