Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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).
Comment | File | Size | Author |
---|---|---|---|
#22 | shiny-add-proudly-built-in-preprocess-1789672-22.patch | 1.46 KB | jsacksick |
#17 | shiny-footer_region_update-1789672-17.patch | 5.72 KB | dudenhofer |
#16 | shiny-footer_region_update-1789672-16.patch | 5.08 KB | dudenhofer |
#13 | creditoptions-1789672-13.patch | 1.81 KB | dudenhofer |
#13 | theme-settings.php_.txt | 693 bytes | dudenhofer |
Comments
Comment #1
pedrorocha CreditAttribution: pedrorocha commentedThe 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).
Comment #2
Ignigena CreditAttribution: Ignigena commentedI'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.
Comment #3
bojanz CreditAttribution: bojanz commentedYou're free to remove it manually from the template file until we solve it properly.
Comment #4
pedrorocha CreditAttribution: pedrorocha commented@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.
Comment #5
tgeller CreditAttribution: tgeller commentedIt'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.
Comment #6
tgeller CreditAttribution: tgeller commented...and here's the patch.
Comment #7
tgeller CreditAttribution: tgeller commentedI'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.
Comment #8
tgeller CreditAttribution: tgeller commentedComment #9
bojanz CreditAttribution: bojanz commentedI 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.
Comment #10
Bojhan CreditAttribution: Bojhan commentedGiven 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.
Comment #11
pedrorocha CreditAttribution: pedrorocha commentedI 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!
Comment #12
tgeller CreditAttribution: tgeller commentedThanks, 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.
Comment #13
dudenhofer CreditAttribution: dudenhofer commentedI 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.
Comment #14
DamienMcKennaThe patches need to be updated to match the Drupal coding standards, but functionally is the correct way to go.
Comment #15
davidwbarratt CreditAttribution: davidwbarratt commentedWe could also just make a simple child theme (of this theme) that has the attribution removed...
thanks!
Comment #16
dudenhofer CreditAttribution: dudenhofer commentedI 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.
Comment #17
dudenhofer CreditAttribution: dudenhofer commentedThis is just the same as the above patch, but targets the default "powered by Drupal Commerce" block instead of using a custom block.
Comment #18
jsacksick CreditAttribution: jsacksick commentedI just tested in Kickstart, and it looks good along with the patch from #1873206: Change Commerce Guys ad from hard-coded to block
Comment #19
jsacksick CreditAttribution: jsacksick commentedIt has been commited, update the issue status.
Comment #20
Paul Lomax CreditAttribution: Paul Lomax commentedThere 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?
Comment #21
dudenhofer CreditAttribution: dudenhofer commentedWe 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.
Comment #22
jsacksick CreditAttribution: jsacksick commentedComment #23
dudenhofer CreditAttribution: dudenhofer commentedThis seems to work great, thanks!
Comment #24
dudenhofer CreditAttribution: dudenhofer commentedSorry @Paul Lomax, i was wrong. thanks jsacksick. Committed here: http://drupalcode.org/project/shiny.git/commit/d185992