Right now the logo included in the theme is the Kickstart logo.

Let's replace that with a druplicon (the one from Seven?), and add the Kickstart logo somewhere in Kickstart, then point towards it when the admin theme is set during install (should be possible since the logo location is a setting. We need to confirm before making the logo change, of course).

Comments

pedrorocha’s picture

The idea is to maintain the logo in the tpl file or maybe change it to template.php, as a variable? It would make easier for other themes to have Shiny as a base theme(i'm using it this way, to create each site admin theme, but now i'm replacing the page.tpl.php file).

Ignigena’s picture

I'd also like to see the option to remove the "proudly built by Commerce Guys" logo if possible. Especially when used in the administrative overlay this just gets in the way and doesn't contribute to the administration process at all.

bojanz’s picture

You're free to remove it manually from the template file until we solve it properly.

pedrorocha’s picture

@bojanz, what do you mean by properly? If you guys have this clear, i can make a patch.

ps: I think that the "developed with Drupal Commerce" is worst than the logo itself, by the way, because the theme will not be used only with Drupal Commerce.

tgeller’s picture

Title:Remove the Kickstart logo» Remove the Kickstart logo and "Proudly built by Commerce Guys"

It's not unusual for themes to include a "built by" text in them; doing so in this theme is no different.

However, its place as the default administration theme for Commerce Kickstart -- and one company's control of both projects -- means it deserves special treatment. Consider: How would you feel if Bartik had "Built by Acquia" hard-coded into Drupal core? Same thing. Would you contribute to the "community" project as enthusiastically?

Considering the need for "special treatment", I've posted a new issue on the Commerce Kickstart project, at #1873206: Change Commerce Guys ad from hard-coded to block. I'll post a patch that's specific to Shiny here, although the best solution -- to move that credit into a block -- will have to be implemented in the Commerce Kickstart issue.

But in any case, that "Built by me!" message has no place in project with dozens of contributors. I hope that you accept my patch right away, without waiting for a resolution on the other issue.

tgeller’s picture

StatusFileSize
new1 KB

...and here's the patch.

tgeller’s picture

StatusFileSize
new1014 bytes

I've had a slight change of heart. Rather than remove *all* credit from Commerce Guys, I shifted it to be clear that they get credit for the Shiny theme. That should be valid for both the Shiny project and Commerce Kickstart.

Use this patch instead.

tgeller’s picture

Status:Active» Needs review
bojanz’s picture

I am fine with #6 and will commit it after lunch.

The point of the credits was to decorate the Kickstart demo store, not brand everything we build.

Bojhan’s picture

Status:Needs review» Reviewed & tested by the community

Given that this is the theme, and not the distribution. I'd also think removing makes more sense, you can add branding like this through the distro. Keep in mind Bartik was not build by Acquia, at all :) This is build by CG, however as you state correctly branding it in the theme is not necessary.

#6 is RTBC.

pedrorocha’s picture

I don't know if it still the same, but if i'm not mistaken, Drupal Commons used to keep this kind of "credits" in a variable in the distro files, so that we could easily remove in the theme, in template.php. It's a simple way to keep things too.

Good to see Shiny improving!

Happy holidays for you guys!

tgeller’s picture

Thanks, folks. I know this is the sort of thing that can cause hurt feelings, and I'm sorry if I was overly forceful. I'm glad to see we're basically in agreement, and really appreciate your efforts.

dudenhofer’s picture

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new693 bytes
new1.81 KB

I don't think just removing this is a 'fix' for this issue. This causes other issues - like the removal of the logo on the installation page. It is somewhat tricky to edit the installation theme, the approach we took here was to create a maintenance page template in the admin theme that is being used on the installation. I could be wrong, but I'm not sure that I see a scenario where the admin theme's maintenance page is displayed other than this installation step. Any other time it would use the default theme like Omega Kickstart.

Another issue is that these 'credits' were placed in a footer wrapper which positioned them outside of the page frame when an overlay was enabled. So at the very least we will need to create a region for placing those blocks or update the stylesheets to replicate this. So we will still need some theme updates that are more than just removing this code.

Maybe we add some theme settings that allow users to turn this off/on. This is similar to pedrorocha's comment but doesn't completely remove this from the theme like Bojhan suggested. But it would be an easy way to remove these credits without requiring people to hack into the code. I've attached a patch + a theme settings file that could make this work if anyone things this might be a good option.

DamienMcKenna’s picture

Status:Needs review» Needs work

The patches need to be updated to match the Drupal coding standards, but functionally is the correct way to go.

davidwbarratt’s picture

We could also just make a simple child theme (of this theme) that has the attribution removed...

thanks!

dudenhofer’s picture

Status:Needs work» Needs review
StatusFileSize
new5.08 KB

I worked out how to get the footer region to display in the overlay and made a patch to replace the coded footer logos with a region. This will need to be updated in Commerce Kickstart to make these blocks. This doesn't address the logo in the installation/maintenance page, but I'm not sure what else we can do about that.

dudenhofer’s picture

This is just the same as the above patch, but targets the default "powered by Drupal Commerce" block instead of using a custom block.

jsacksick’s picture

Status:Needs review» Reviewed & tested by the community

I just tested in Kickstart, and it looks good along with the patch from #1873206: Change Commerce Guys ad from hard-coded to block

jsacksick’s picture

Status:Reviewed & tested by the community» Fixed

It has been commited, update the issue status.

Paul Lomax’s picture

Status:Fixed» Needs work

There is still a reference to this in the maintenance-page.tpl.php

Can't really replace it with a block region, could just be removed?

dudenhofer’s picture

Status:Needs work» Fixed

We were just talking about this before closing this issue. As far as I know, the maintenance page is only used on the installation. Any other time, the front end theme's maintenance page should be used. Since this is the theme that is being used for the Commerce Kickstart installation, it makes sense to leave it for now since it will only be seen on an installation of Commerce Kickstart.

We can open a separate issue for the maintenance page specifically if/when we find a better solution for that.

jsacksick’s picture

Status:Fixed» Needs review
StatusFileSize
new1.46 KB
dudenhofer’s picture

Status:Needs review» Reviewed & tested by the community

This seems to work great, thanks!

dudenhofer’s picture

Status:Reviewed & tested by the community» Fixed

Sorry @Paul Lomax, i was wrong. thanks jsacksick. Committed here: http://drupalcode.org/project/shiny.git/commit/d185992

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.