add download link for modules, themes and translations
Pasqualle - September 15, 2007 - 14:10
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | system.module |
| Category: | feature request |
| Priority: | minor |
| Assigned: | patchnewbie |
| Status: | closed |
Description
I think it would be good to provide download link for modules and themes on administration pages (admin/build/modules, admin/build/themes, admin/logs/updates)
reason:
The download links are on welcome page (after the installation), there are also links on page "available updates" (admin/logs/updates), but there are no links (in administration section) to download new modules and themes.
This could encourage people to use (or just try) additional modules and themes with drupal.
#1
+1
It would be very convenient to have a link to modules and themes from their respective admin pages.
#2
It could be as simple as
Additional modules are available from drupal.org
I'd recommend putting that line below the line regarding "available updates" for the module list.
#3
This would be a great patch for a new contributor to get started with core development.
#4
I've had a go at patching this. Thanks for opening up core development for noobs like myself patchnewbie :D.
I modified these pages: admin/build/modules, admin/build/themes, admin/logs/updates.
One thing that needs work is that I don't know how to get the appropriate text to appear *after* update.module's implementation of hook_help(). If this were possible then the added notification could be the last thing to appear before the important content on the page.
Update.module patch coming soon.
#5
Here's the patch for update.module.
#6
hazexp - Great job as a patch newbie! I only have two comments
+ $output = '<p>'. t('To change the appearance of Drupal have a look at the <a href="http://drupal.org/project/Themes">contributed themes</a> available at <a href="http://drupal.org/">drupal.org</a>.').'</p>';We actually don't use tabs in our markup. You should use two spaces.
Secondly,
$output .= t('<p>It is important that <a href="@update-php">update.php</a> is run every time a module is updated to a newer version.</p><p>You can find all administration tasks belonging to a particular module on the <a href="@by-module">administration by module page</a>.</p>', array('@update-php' => $base_url .'/update.php', '@by-module' => url('admin/by-module')));+ $output .= '<p>'. t('Please note that only core functionality is included by default. To extend Drupal\'s functionality see the <a href="http://drupal.org/project/Modules">contributed modules</a> at <a href="http://drupal.org/">drupal.org</a>').'</p>';
The t() is inconsistant. In the first one, can you putu the
<p>outside thet?#7
#8
Ok, I've updated the patches for better consistency and convention conformity.
Here's the system.module patch. Update to follow.
#9
Here's the update.module patch.
#10
Looks great! And welcome to the wonderful world of patches!
#11
@hazexp - Good job for your first try at patching and keep up the good work!
I've combined and rerolled the patch using CVS diff rather than plain diff. I also changed some wording to be more generic, removing references to Drupal; moved the links to an array so they are properly escaped and are easier for translators to work with; and broke the line with the <p> into separate lines.
#12
I would remove the
at <a href="http://drupal.org/">drupal.org</a>part. One can select 'Home' to go to the home page.#13
Patch #11 applies cleanly, works as advertised, worded nicely IMO, and is a huge usability improvement for new users struggling with their first site, helping them to find things quickly. So setting RTBC.
As for #12 - the patch #11 doesn't have this in it, already.
#14
Wow HazeXP, this moved rather faster than I thought it would, I had been discussing this with Webchick and Amazon a few days ago, and hadn't been able to revisit it since.
Nice job on the patch, I prefer your wording to what I had crafted originally.
A related enhancement we may want to consider, is to include form arguments in that link to filter the available releases based on the major version of your drupal install.
This would require a patch to project.module from what I understand, changing the quick navigation form method to 'GET'.
#15
I agree this is a good idea. But let's not forget translations (http://drupal.org/project/Translations). Locale module has a language setup page which would be enhanced by a link to translations. So we cover most project types (install profiles will obviously not be suggested on a site already set up).
#16
I had a problem to understand you Gábor. It could be my poor english. So just made a sum up of your request.
task: add link http://drupal.org/project/Translations to admin page admin/build/translate
anything else need a download link?
#17
Yes, but I would add the link on the language setup page too:
/admin/settings/language
/admin/build/translate
Note that /admin/build/translate/import already has the link, but it seems we need to fix it for case (ie. Translations instead of translations in the URL).
#18
It seems unfair 3 download links for translations and 1-1 for modules and themes. We can't add translation download link to every admin page, sorry. :)
any suggestions?
#19
Actually language features span more pages then module or theme listings. But I am convinceable with something more solid then "we need equality :)".
#20
The link works, but yes, that should be fixed. Here is the explanation http://drupal.org/node/115092
#21
Added the Translations links and did some minor cleanup of the locale help text.
#22
Just a note to hazexp: Notice how the anchors in recent patches use
@variablesin place of URLs, and defines them afterwards. :)#23
Just a tiny edit to #21 patch: Unnecessary double quotes replaced with single quotes (admin/settings/language/add - t() replacement array).
Patch applies cleanly, works fine, looks fine... So setting RTBC, as I didn't really change the patch in any significant way.
#24
Created a feature request (http://drupal.org/node/185698) against project.module to modify the filter form so we can pass a version argument to it from these links.
#25
Hold on here...
A) I positively *hate* the fact that Translations and Modules are capitalized like that in those URLs, and would like to fix that some day. Once we hard-code those into core, our hands are tied forever. :( http://drupal.org/node/115092 shouldn't be at "by design", it should be active. ;)
B) The feature request about version-specific download links is duplicate with http://drupal.org/node/107813 which, IMHO, is a much better solution to that problem.
So, I'd be opposed to this patch going into core as it currently stands. Moreover, I'd appreciate some help with these project*-related issues, since my hands are full with preliminary work for the D6 porting effort. Thanks.
#26
@dww: http://drupal.org/project/modules and friends sort of work, although the page title and the tabs are not there, the page shows proper contents. So we can just as well link to that and get these bugs fixed (I understand you would like to have these bugs fixed first in project module). Translations already link to the lowercase URL, which is not a problem there as there are no tabs on that page (and interestingly the page title is also OK).
#27
What is the consensus on this now? This is a good usability feature and it would be great to get this in the first RC.
#28
I think it's a good idea. Also makes sense to link to lowercase version of urls and fix in project later.
#29
Patch is RTBC then, it still applies cleanly.
#30
patch goes to uppercase version of links. seems like dww is very much against that (I also hate uppercase text in links so +1 to changing). With lowercase links it'd be rtbc though
#31
Rerolled - changing all links to /Themes and /Modules to the lowercase versions.
Untested.
#32
Forgot /Translations
Untested.
#33
A couple of minor things that may not be strictly the fault of this patch, but since this is modifying those lines anyway (note that there is another patch in the documentation queue -- http://drupal.org/node/163246 -- that has a couple of these changes in it already -- if this gets in, that can be rerolled):
-- There is an "Alternately" in here that probably should be "Alternatively".
-- There is surely some way to avoid "courses" in this usage:
+ $output .= '<p>'. t('Drupal interface translations may be added or extended by several courses: by <a href="@import">importing</a> an existing translation, by <a href="@search">translating everything</a> from scratch, or by a combination of these approaches.', array('@import' => url('admin/build/translate/import'), '@search' => url('admin/build/translate/search'))) .'</p>';even something as simple as "Drupal interface translations may be added or extended by importing an existing....".
-- In this:
+ $output = '<p>'. t("This page provides an overview of interface translation on the site. Drupal groups all translatable strings in so called 'text groups'. Modules may provide more text groups additionaly to the 'built-in interface' default, to separate strings used for other purposes, allowing you to focus your translation efforts on the groups of text you care most about. For example, a translation team could choose not to fully translate a text group that includes less important data, being still able to ensure that built-in interface is always fully translated.") .'</p>';"additionaly" is probably a typo.
This entire string could use some tweaking, but may be out of scope for this patch.
#34
OK quick edit on those strings.
#35
Tested the patch in #34, all applies well.
#36
I still think that Alternately should be Alternatively:
- return '<p>'. t('Select which themes are available to your users and specify the default theme. To configure site-wide display settings, click the "configure" task above. Alternately, to override these settings in a specific theme, click the "configure" link for the corresponding theme. Note that different themes may have different regions available for rendering content like blocks. If you want consistency in what your users see, you may wish to enable only one theme.') .'</p>';+ $output = '<p>'. t('Select which themes are available to your users and specify the default theme. To configure site-wide display settings, click the "configure" task above. Alternately, to override these settings in a specific theme, click the "configure" link for the corresponding theme. Note that different themes may have different regions available for rendering content like blocks. If you want consistency in what your users see, you may wish to enable only one theme.') .'</p>';
#37
Another quick edit, missed that one.
Please note don't credit me for this patch if it's committed.
#38
I asked dww to comment on this issue. While I am all happy with the changes, it would be good to know whether the URL lowercasing is in the plans for the Drupal 6 project upgrade, so we are fine with wiring these URLs in. It is of note however that the uppercase ones were there before (possibly even in D5, I have not looked). Anyway, the lowercase ones are now sort of working but provide reduced functionality, so we need to ensure that they will work fine.
#39
Yes, the lower case paths will be working fine on d.o before 6.0 roles out. Haven't looked at the current patch but I'll assume the other reviews are accurate and this is still RTBC.
Cheers,
-Derek
#40
Great, committed, thanks all.
#41
Automatically closed -- issue fixed for two weeks with no activity.