This issue is the obvious followup to #1446650: After installing module display link to "install another module".

Problem/Motivation

Make installing another theme easier by linking to the install page after a new theme is installed.

Proposed resolution

Add a "Download another theme" "Install another theme" link after a new theme is installed.

Also, to match setup of core/lib/Drupal/Core/Updater/Module.php, set up row of links in the following order:

  • Install another theme
  • Enable newly added theme
  • Administration pages

Remaining tasks

Reviewed and Tested by the Community.

From core/lib/Drupal/Core/Updater/Module.php:

 /**
   * {@inheritdoc}
   */
  public function postInstallTasks() {
    // Since this is being called outsite of the primary front controller,
    // the base_url needs to be set explicitly to ensure that links are
    // relative to the site root.
    // @todo Simplify with https://www.drupal.org/node/2548095
    $default_options = [
      '#type' => 'link',
      '#options' => [
        'absolute' => TRUE,
        'base_url' => $GLOBALS['base_url'],
      ],
    ];
    return [
      $default_options + [
        '#url' => Url::fromRoute('update.module_install'),
        '#title' => t('Install another module'),
      ],
      $default_options + [
        '#url' => Url::fromRoute('system.modules_list'),
        '#title' => t('Enable newly added modules'),
      ],
      $default_options + [
        '#url' => Url::fromRoute('system.admin'),
        '#title' => t('Administration pages'),
      ],
    ];
  }
Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Update the issue summary Instructions Done
Manually test the patch Novice Instructions Done
Embed before and after screenshots in the issue summary Novice Instructions Done

User interface changes

Before:

After: after screenshot

API changes

None

CommentFileSizeAuthor
#109 Screen Shot 2017-01-28 at 5.25.58 PM.png66.9 KBmspangenberg
#107 1862094-2.patch1.75 KBmspangenberg
#106 Screen Shot 2017-01-28 at 4.40.40 PM.png75.86 KBmspangenberg
#104 1862094 - after.png37.58 KBsteverossnyc
#104 1862094 - before.png34.73 KBsteverossnyc
#102 patch update test with php7.png80.81 KBikit-claw
#101 1862094.patch1.73 KBcleverhoods
#100 patch test with php7.png82.9 KBikit-claw
#98 1862094-96.patch2.26 KBcleverhoods
#95 8x4withpatchBOOSTRAPtheme.png247.24 KBcleverhoods
#91 1862094-91.patch1.94 KBmanishmittal9
#89 1862094-88.patch1.94 KBmanishmittal9
#83 interdiff-80-83.txt441 bytesThew
#83 1862094-83.patch1.39 KBThew
#80 interdiff-77-80.txt1 KBThew
#80 1862094-80.patch1.39 KBThew
#77 install-another-theme-1862094-77.patch988 byteslomasr
#69 enable-theme-button.jpeg26.39 KByoroy
#61 Screen Shot 2016-05-13 at 3.44.34 PM.png55.58 KBjfraz
#60 Screen Shot 2016-05-13 at 3.28.29 PM.png72.58 KBjfraz
#59 interdiff.txt568 bytessnehi
#59 install-another-theme-1862094-59.patch1.13 KBsnehi
#57 install-another-theme-1862094-57.patch1.14 KBsaurabhsirdixit
#52 After_patch.JPG25.07 KBTruptti
#52 Before_patch.JPG28.34 KBTruptti
#49 install-another-theme-1862094-49.patch1.13 KBManjit.Singh
#48 Update-manager-after.png82.14 KBManjit.Singh
#48 Update-manager-before.png78.51 KBManjit.Singh
#43 Screenshot-after-installing-new-theme.png354.61 KBswetashahi
#41 install-another-theme-1862094-41-8.patch1.01 KBhugronaphor
#23 install-another-theme-1862094-23.patch572 byteskallehauge
#18 install-another-theme-1862094-18.patch658 bytesalimac
#13 interdiff-1862094-11-13.txt615 bytesalimac
#13 install-another-theme-1862094-13.patch622 bytesalimac
#11 install-another-theme-1862094-11.patch544 bytesalimac
#9 install-another-theme-1862094-9.patch512 bytesalimac
#1 install-another-theme-1862094-1.patch511 bytesDavid_Rothstein

Issue fork drupal-1862094

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David_Rothstein’s picture

Status: Active » Needs review
FileSize
511 bytes

And an obvious (but untested) patch.

benjifisher’s picture

Status: Needs review » Reviewed & tested by the community

I tested it, and it works as expected. Luckily, there are a few modules with 8.x-?.x-dev releases available for experimentation. I did not try enabling the themes after downloading them.

I think being able to install multiple themes is less valuable than being able to install multiple modules, but this patch is good for consistency.

Shameless plug: my patch at #720452: Allow ability to install multiple modules at once could use some attention.

catch’s picture

Status: Reviewed & tested by the community » Needs review

I'm not sure it's that obvious. Most sites have dozens of modules installed, but only one theme.

David_Rothstein’s picture

Well, if you are downloading themes from drupal.org (rather than building your own) most likely you will want to try out a few before settling on a final one.

benjifisher’s picture

Status: Needs review » Needs work

The last submitted patch, 1: install-another-theme-1862094-1.patch, failed testing.

benjifisher’s picture

Issue summary: View changes
Issue tags: +Novice, +Needs reroll

The patch no longer applies. This should be an easy re-roll, so I am adding tags.

alimac’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll +Amsterdam2014
FileSize
512 bytes

Re-rolled.

alimac’s picture

+++ b/core/lib/Drupal/Core/Updater/Theme.php
@@ -82,6 +82,7 @@ public function postInstall() {
+      l(t('Install another theme'), 'admin/appearance/install'),
       l(t('Install newly added themes'), 'admin/appearance'),

Looking at these two options, they're a bit confusing ("Install another theme vs. Install newly added themes"). Does "Install newly added themes" mean "Enable"?

alimac’s picture

Did not apply to HEAD, so I rerolled it again.

benjifisher’s picture

The issue summary points out that this is a follow-up to the corresponding issue for installing modules, #1446650: After installing module display link to "install another module". The language used there is

+      l(t('Install another module'), 'admin/modules/install'),
       l(t('Enable newly added modules'), 'admin/modules'),
       l(t('Administration pages'), 'admin'),

I agree with the suggestion in #10 of changing the text on the existing link from "Install" to "Enable".

alimac’s picture

Here's an updated patch which makes the link name clear and consistent.

benjifisher’s picture

@alimac:

The interdiff does not look right. Can you check it and post again? Specifically, it is not consistent with the updates from #9 tp #11, replacing l() with \Drupal::l() and replacing D7-style paths with D8-style routes.

alimac’s picture

@benjifisher: the interdiff is for 11 to 13. I did not provide an interdiff for 9 to 11 because it was a reroll (I rebased, then resolved the conflicting lines).

benjifisher’s picture

Status: Needs review » Reviewed & tested by the community

@alimac:

Sorry, I must have been looking at the wrong interdiff.

I am marking this RTBC. I already did that way back in #2. The only objection has been @catch's comment in #3. This is not at all a technical question, it is a UX question. In my opinion, consistency with the modules page is a good enough reason to include this patch. (Even though I am not at the Amsterdam code sprint, I took the trouble to pull the latest code from git and check that it is, indeed, consistent.)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: install-another-theme-1862094-13.patch, failed testing.

alimac’s picture

Status: Needs work » Needs review
FileSize
658 bytes

Rerolled for latest HEAD.

benjifisher’s picture

Status: Needs review » Reviewed & tested by the community

@alimac:

It is hard to hit a moving target! The patch still looks good to me.

See my comments in #16.

alimac’s picture

Issue summary: View changes
alimac’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/lib/Drupal/Core/Updater/Theme.php
@@ -84,7 +84,8 @@ public function postInstall() {
+      \Drupal::l(t('Install another theme'), new Url('system.theme_install')),
+      \Drupal::l(t('Enable newly added themes'), new Url('system.themes_page')),

Hmmm... the new link here is wrong. It should be to update.theme_install. Also I think we have a problem with what install means. Since what you can do on that route is download a new theme. And all the usage of the word "enable" has been removed.

kallehauge’s picture

I agree with @alexpott at #22 and suggest that we keep the original link that was modified in patch #18 by @alimac and simply add a new link pointing to "update.theme_install" with the string "Download another theme".

alimac’s picture

Status: Needs work » Needs review

@kallehauge: thanks for posting a new patch. I'm changing the status of the issue to Needs review so that the testbot can automatically test it.

alimac’s picture

This is a small patch, so it's easy to "eyeball" the difference, but it's really helpful to post an interdiff between a patch and the previous one, which shows the difference between them. Here's how to make one (for future reference): https://www.drupal.org/documentation/git/interdiff

kallehauge’s picture

I was actually about to post an interdiff, but considering the amount of code that was changed in the patch, I actually felt it would be more confusing than helpful in this scenario :) But I will keep it in mind! Thanks @alimac!

Status: Needs review » Needs work

The last submitted patch, 23: install-another-theme-1862094-23.patch, failed testing.

vlad.n’s picture

install-another-theme-1862094-23.patch patch applied cleanly at f07bcff.

rpayanm’s picture

Status: Needs work » Needs review

Testbot random failures :(

alimac’s picture

I think we might have to mark this as postponed, due to: Remove the UI for installing/updating modules from update module if it is not fixed in time for release.

Installing themes (and modules) through the web UI doesn't work currently, so the patch for this issue cannot be tested.

alimac’s picture

Issue summary: View changes
David_Rothstein’s picture

Status: Postponed » Needs review

This is marked for Drupal 7 backport so I don't think it should be postponed.

To test it manually on Drupal 8, just apply it on top of the latest patch from #2042447: Install a module user interface does not install modules (or themes).

kfitz’s picture

While attempting to install themes to test this patch, on a virtual host, I kept receiving the error: 'theme_name' was not found. This was case even when attempting install the following:

Adminimal Theme
Omega Theme
Adaptive Theme

I was able to successfully apply the following two patches:
https://www.drupal.org/node/2042447
#23

I set permissions on themes to 777 and unpacked the themes into the themes folder after downloading them. Additionally, I was not able to install themes via providing a url or path to a zip/tar. I'm not sure if there is something I'm missing, or the D8 API is not stable enough to manually test this issue, but it would be nice to see it make it into 8.0.x.

rutgers03’s picture

Assigned: Unassigned » rutgers03

Working on this at Drupal Camp NJ

rutgers03’s picture

Assigned: rutgers03 » Unassigned
swetashahi’s picture

Status: Needs review » Needs work

Uable to test using the patch in #23 in simplytest.me. Got an error saying "An error occurred while patching the project." Marking Needs work

hugronaphor’s picture

Here is the path applicable to the latest 8.0.x and 8.1.x branches.
No interdiff as core implementation changed since the previous patch.

hugronaphor’s picture

Status: Needs work » Needs review

Triggering testbot.

swetashahi’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
354.61 KB

I tested the patch in #41 using SimplyTest .me on chrome. here are the steps followed while testing:

1. Downloaded theme from drupal.org site from https://www.drupal.org/node/2638078
2. Go to Appearance -> Install a new theme
3. Browse to the downloaded zip file. Click Install
4. Installation happens successfully and the following message appears under "Next steps"
"Download another theme
Install newly added themes
Administration pages"
Screenshot here

image

Clicking on Download another theme takes to the Install new theme screen. Here the newly installed theme in step 3, (surprisingly) appears under uninstalled themes but again clicking install moves it to installed themes section in a second.

This is as per the proposed resolution. Marking RTBC

catch’s picture

Status: Reviewed & tested by the community » Needs review

It seems more likely that you'd want to install the theme you just downloaded than go install another one - so shouldn't this be the second rather than first option in the list?

swetashahi’s picture

But in the ticket description the proposed resolution was having a link for Installing another theme.

If the issue is about the ordering of text then it should be a separate ticket.

catch’s picture

Status: Needs review » Needs work

@swetashahi it's adding the link where there was previously no link and introducing the ordering issue by doing so - that makes it a problem with the patch here, not a separate issue.

David_Rothstein’s picture

Version: 8.0.x-dev » 8.1.x-dev

I think this should be made exactly consistent with how the module links work (which is what I tried to do in the original patch in #1).

Since the module links are currently:

  • Install another module
  • Enable newly added modules
  • Administration pages

The theme links should be:

  • Install another theme
  • Enable newly added themes
  • Administration pages

If we want to reorder the links it needs to be done for modules too, but I think it might be a bit out of scope for this issue.

And "Download another theme" seems like it would be pretty confusing - what you're doing from this page is actually much closer to "uploading" than "downloading".... There are already other issues in the queue that talk about changing the terminology around this, so I don't think we need to get into it here.

Manjit.Singh’s picture

Here is my observations Guys.

I have test the latest patch. Please check attached screenshots.

Before
Before

After
After

@Catch The position/placement of Download link is at top. And we have to modify the Issues description a bit for better understanding as @David mentioned above.
Or Can we create a separate ticket for it ?

Manjit.Singh’s picture

Status: Needs work » Needs review
FileSize
1.13 KB

@David reordering links as per your feedback.

David_Rothstein’s picture

Thanks - I didn't test the new patch, but from looking at it aren't they still in an inconsistent order compared to the links for modules?

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Truptti’s picture

FileSize
28.34 KB
25.07 KB

Verified the patch ' install-another-theme-1862094-49.patch'' in #49 on drupal 8.1.x, also the same patch applied successfully on drupal 8.2.x site .After installing theme the links on success screen are displayed as below

  • Enable newly added themes
  • Install another theme
  • Administration pages

Attaching snapshot for reference.

Truptti’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs usability review, +Needs product manager review

Sites are expected to have tens of modules installed, sometimes into the hundreds.

They're only expected to have two themes, more than that is for extremely rare special cases.

So these are not equivalents, and treating non-equivalents the same is not something we do elsewhere - i.e. the modules and appearance pages are not the same either.

Tagging 'Needs usability review' and for product review, but I'm not personally going to commit this as is.

yoroy’s picture

I did raise my eyebrows at this for the reasons @catch gives. It's not at all obvious to me that because the modules screen has this then appearance page should have it too. It would be less of a problem if we had stronger visual hierarchy among these links, we'd make the "install" link the primary one and the other two would serve the less important scenarios. Because we don't have that hierarchy here this link adds friction: choosing between 3 instead of 2 links is more work for the user. For those who do want to install the next theme right away, it's only one more click to the Appearance page (via "install newly added" link) where you can choose to "install new theme" again.

So, this is a worthwhile shortcut if installing multiple things is the more likely scenario. That does apply to modules, but not to themes. Lets not do this.

(I'm collecting visual hierarchy issues under the "needs design" tag)

David_Rothstein’s picture

Status: Needs review » Needs work

The screenshots in #52 show that the patch still puts this link in the wrong place, so marking "needs work" for that. (We should create a separate issue to discuss whether "enable" belongs before "install" more generally, i.e. in the module case.)

I really don't understand why people think this link wouldn't be useful. To repeat my comment from #4:

Well, if you are downloading themes from drupal.org (rather than building your own) most likely you will want to try out a few before settling on a final one.

That really seems like the most likely scenario to me - you've found a few themes that look potentially interesting and you want to download and try them all out.

On top of that there's the issue of base themes - a decent number of contrib themes depend on a base theme, so if you're downloading one of those then even if you do already know that's the final theme you want to use you'll still need to go back and download a second one.

For those who do want to install the next theme right away, it's only one more click to the Appearance page (via "install newly added" link) where you can choose to "install new theme" again.

Yes, it's only one extra click, but I'm not sure it's an obvious click for new users.

saurabhsirdixit’s picture

Status: Needs work » Needs review
FileSize
1.14 KB

Altered the links as per comment #56 to make similar experience with module screen.

snehi’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Core/Updater/Theme.php
@@ -105,8 +105,12 @@ public function postInstallTasks() {
+      ],    ¶

Empty spaces here. I don't know are you placing it on correct place.

snehi’s picture

Status: Needs work » Needs review
FileSize
1.13 KB
568 bytes
jfraz’s picture

Issue summary: View changes
FileSize
72.58 KB

I spun this up on simplytest.me and verified that the link indeed exists and directs the user to the page to install another theme.

after patch applied screenshot

jfraz’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update
FileSize
55.58 KB

I've reviewed this and feel it works as intended. I normally build my own themes, so this isn't something I would use. But I do see the applicable use for someone who is testing various themes. Updated issue summary. This was reviewed for training at DrupalCon New Orleans.

jfraz’s picture

Issue summary: View changes
hugronaphor’s picture

Please stop testing or uploading new patches for this issues.
Someone needs to decide either we need this "install another theme" link or not!
See: #54 and #55

catch’s picture

Yoroy already weighed in. But putting back in the usability queue one more time, and leaving the product review tag on as well.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 59: install-another-theme-1862094-59.patch, failed testing.

Manjit.Singh’s picture

@catch @codethenode the link "Enable newly added themes" is taking the user to appearance page to enable the particular theme. But as per the usability, the link should enable the newly added theme rather redirecting the user to appearance page.

snehi’s picture

@manjit right now how are you enabling theme.
You have to first go to the appearance page and then enable it. I think it is ok.

Different way of enabling theme either use drush or drupalconsole.

thanks

David_Rothstein’s picture

Status: Needs work » Needs review
yoroy’s picture

Issue tags: -Needs usability review
FileSize
26.39 KB

Yes, it's only one extra click, but I'm not sure it's an obvious click for new users.

Well, without the link for "install another" there's only two options, either go to the Appearance page or to the top level admin page :-)
I wouldn't be so against this if we could visually promote the "Enable newly added" one:

Edit: Forgot to mention that base theme + sub theme is a good argument in favor of adding the link.

David_Rothstein’s picture

I like it, but the button makes it look like clicking on it will actually enable the theme (or at least take you to a dedicated page for doing that), so maybe it should be combined with or postponed on #992190: Link to enable a new module after adding it via the Update Manager is confusing - allow users to enable it directly from that screen instead? But one way or another it is definitely a good idea to highlight that action over the others.

yoroy’s picture

You're right, this would not the correct styling, forgot to mention that. I don't think we currently have a primary link (not button) style, but we can make one.

Bojhan’s picture

Lets have product manager review, before moving further on this patch to avoid wasting time.

webchick’s picture

This is like a 1% screen. I don't think it needs product management review.

However, my 2 cents is that #69 seems like it'd make sense for both modules and themes, and would both address David's concern about inconsistency, and catch's concern about promoting a non-default action as the default.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

@yoroy Maybe you could re-title this issue to re-focus it? It seems like there is agreement with the proposed new link suggestion.

joelpittet’s picture

Issue tags: +Usability

bat signal

lomasr’s picture

Hi,

As suggested in #69 , I have created a patch. Please review.

Thanks

Status: Needs review » Needs work

The last submitted patch, 77: install-another-theme-1862094-77.patch, failed testing.

bhavikshah9’s picture

Patch#77 failed. As well as, I guess, indentation is in-correct.

Thew’s picture

The patch from #77

  • Fix indentation
  • Comment spelling
Thew’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 80: 1862094-80.patch, failed testing.

Thew’s picture

Status: Needs work » Needs review
FileSize
1.39 KB
441 bytes

Fix more indentation.
Hope it pass.

Status: Needs review » Needs work

The last submitted patch, 83: 1862094-83.patch, failed testing.

benjifisher’s picture

Did you test the patch at all? I would be surprised if it works.

     return [
+      $url = Url::fromRoute('system.themes_page'),
+      $link_options = array(
+        'attributes' => array(
+          'class' => array(
+            'button',
+          ),
+        ),
+      );
+      $url->setOptions($link_options);
       $default_options + [
-        '#url' => Url::fromRoute('system.themes_page'),
-        '#title' => t('Install newly added themes'),
+        '#url' => $url,
+        '#title' => t('Enable theme'),
+      ],
+      $default_options + [
+        '#url' => Url::fromRoute('update.theme_install'),
+        '#title' => t('Install another theme'),
       ],
       $default_options + [
         '#url' => Url::fromRoute('system.admin'),

You need to move the assignments and the setOptions() lines outside the return statement.

Also, this patch mixes the old array('foo') syntax with the newer ['bar'] syntax. According to the coding standards,

The use of PHP 5.4+ short array syntax for Drupal 8.x is being discussed and is used in some patches already. When used, try to apply it consistently to at least a whole method or function.

benjifisher’s picture

I think a cleaner patch (untested) would be to assign

$button_style = ['attributes' => ['class' => [' button']]];

outside the return statement. Then, inside the array being returned, update the link text and apply setOptions($button_style) to the appropriate URL.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

manishmittal9’s picture

Assigned: Unassigned » manishmittal9
manishmittal9’s picture

Assigned: manishmittal9 » Unassigned
Status: Needs work » Needs review
FileSize
1.94 KB
manishmittal9’s picture

1862094-88.patch Updated Patch

manishmittal9’s picture

FileSize
1.94 KB

Forgot to upload the patch in earlier comment.

cleverhoods’s picture

I'm going to review it

ifrik’s picture

manishmittal9,

Could you post an interdiff as well? That makes the reviewing easier.
https://www.drupal.org/documentation/git/interdiff

ifrik’s picture

Issue tags: +SprintWeekend2017
cleverhoods’s picture

Status: Needs review » Needs work
FileSize
247.24 KB

After applying the patch to a clean 8.4x installation I get the following error.
8.4-test

kusalavan’s picture

Status: Needs work » Needs review
Issue tags: +london_2017_january

I was able to test this successfully and i could see the "Install another theme" link after installing a theme.

ifrik’s picture

Status: Needs review » Needs work

kusalavan,

cleverhoods had put this issue on "needs work" because he encountered the error message above after applying the patch, and he is working on this during today's sprint.

cleverhoods’s picture

Fixed the issues reported in #86. Please review the patch

cleverhoods’s picture

Status: Needs work » Needs review
ikit-claw’s picture

FileSize
82.9 KB

Works fine on my setup php7

Could possibly do with a link to install a theme from appearance as well as extend?

cleverhoods’s picture

Added it as a link instead of a button.

ikit-claw’s picture

Updated no longer displayed as button

ikit-claw’s picture

steverossnyc’s picture

FileSize
34.73 KB
37.58 KB

Works on 8.4-dev
Before:
before
After:
after

steverossnyc’s picture

Status: Needs review » Reviewed & tested by the community
mspangenberg’s picture

Status: Reviewed & tested by the community » Needs work
FileSize
75.86 KB

In order to maintain consistency with the Modules Next Steps the order of the operations should be:

  • Install another theme
  • Enable newly added theme
  • Administration pages

See attached image.

mspangenberg’s picture

Updated patch to match verbiage of Modules page.

Did not unit test. Appears functional based on prior test.

mspangenberg’s picture

Added 1862094-2.patch and Screen Shot 2017-01-28 at 4.40.40 PM.png to resolve order of operations issue in verbiage.

Next steps for themes should mirror order of operations of Next steps for modules (see Screen Shot).

mspangenberg’s picture

Tested 1862094-2.patch, screen shot attached, applied successfully.

cleverington’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Reviewed patch and tested.

Updated Issue Summary with new screenshot from mspangenberg.

Issue 1862094-2 patch Screen Shot

cleverington’s picture

Issue summary: View changes

Updated Summary for required alterations

David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs review

This goes against what was discussed in #69 (and what subsequent patches tried to implement). There is no visual distinction between the primary link and the other links.

Since we don't have a primary link styling (see #71) we need to come up with one. I am going to suggest simply making the link bold (i.e. <strong> tags) if no one else has a better idea...

Also the screenshots in #104 and #110 differ and it's not clear which patch is RTBC. The #104 screenshot looks more like #69 (although still missing the styling) - I think the idea is to go with that here, and then ensure that installing a module uses the equivalent wording, link order, and styling as well.

cleverington’s picture

I would hesitate to update the UX/UI on admin/appearance when not making a similar alteration on the same page/functionality within admin/modules.

In truth, this issue appears to be a sibling of https://www.drupal.org/node/1446650

I agree that the ideas discussed in #69 / #71 are a good approach, but I think adding <strong> or an image-button would probably be a separate issue with a relation to https://www.drupal.org/node/992190 as an additional UI/UX enhancement.

The patch reviewed is https://www.drupal.org/files/issues/1862094-2.patch, an small tweak to https://www.drupal.org/files/issues/1862094.patch with UI matching for https://www.drupal.org/node/1446650.

The Screenshot was updated in the Summary.

If going forward with <strong> to re-enforce the primary link, I would recommend creating a sibling issue (unless one already exists I did not find) to https://www.drupal.org/node/992190 and adding the button on both pages at the same time.

Edit: Updated after reading background behind #9292190.

The last submitted patch, 101: 1862094.patch, failed testing.

David_Rothstein’s picture

I mostly agree with you - in fact it's what I suggested on this issue about a year ago :)

However, this new patch has the exact same change that was already rejected before. The UX maintainers felt it would be confusing to add the extra link without also making additional changes.

cleverington’s picture

Ah. I See. #47. Rejected within #54.

So if the recommended alteration is not approved, should the Issue be Closed and a new issue created with the ideas behind #69, #71, and #73? Or altered to support those posts? Closed because neither option is viable Use Cases?

For continuance, it seems like it should be possible to create a small View which loads the content of admin/appearance within a container (or similar) with only the recently installed theme/themes.

An array could track the associated theme(s) that were just installed. Then, upon installation, the User could Enable one of them before being redirected to the admin/appearance screen.

Oddly, thinking about it, the fix seems more.... viable for the modules page altogether, simply because downloading and installing a theme could potentially download multiple themes, such as with Omega, etc. Trying to have the simplest method, a button which 'enables the theme' would fail simply because the array would always be more than [0].

In contrast, on the Modules page, completion would not be much of an issue, because the Module-Filter module is already an existing functionality and replicating the layout would be fairly easy.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

jhodgdon’s picture

jhodgdon’s picture

FYI -- on the parent issue #2888657: [meta] Less confusing and more consistent wording needed in module/theme add/install/update the Usability team is discussing the UI text for this and about 10 other issues. Until decisions are made, making another patch here would probably be premature.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

jhodgdon’s picture

Status: Needs review » Needs work
Issue tags: -Novice

Removing Novice tag for now because we need to make a Usability team decision on how to proceed.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Aditi Saraf made their first commit to this issue’s fork.