The field "Upload logo" on the admin page of the module (/admin/config/user-interface/print) was not changing the print icon even if you upload an image to the field.

So it was found that the path of the icon was hard coded, it was not using the uploaded file.

You can confirm that on line 700 of print.module 7.x-1.3

Print Hardcoded Icon

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mariosr created an issue. See original summary.

mariosr’s picture

FileSize
737 bytes

Uploading patch to fix issue.

mnishida’s picture

Hi @mariosr,

I can review this issue.

Thanks in advance,
Matheus Nishida

davic’s picture

Issue summary: View changes
FileSize
18.25 KB

Updated Issue Summary.

This was related to the variable saved in the settings form not being used afterwards.

The patch is working fine for me, let's wait for @mnishida's revision to make sure it's okay.

mnishida’s picture

Assigned: mariosr » mnishida
mnishida’s picture

Assigned: mnishida » Unassigned
Status: Needs review » Reviewed & tested by the community
Issue tags: +#ciandt-contrib

Hi @mariosr and @davic,

After my performing a series of tests, the patch worked clean.

I'm changing the issue's status to RTBC. If there's any questions, please feel free to ask them any time.

Best regards,
Matheus Nishida

jcnventura’s picture

Status: Reviewed & tested by the community » Closed (works as designed)

This is funny, and so wrong on so many aspects.

That "print_logo_url" is NOT the print icon.. It would be called print_icon_url, if it was that. What that variable does is to change the site logo to be used in the PF version. So this is effectively now using what is supposed to be the print icon on every page (unless of course you changed the template).

If you truly want to change the icon you have a few choices:
1. Write your own version of theme_print_ui_format_link()
2. Enable the text-only version, and use CSS to insert your own logo.

I don't believe that module should support uploading your own icons.. That's the realm of custom themes, and the module already provides more than adequate support for those.

Obrigado a todos ai na CI&T, mas espero que compreendam que este patch não será aplicado.

davic’s picture

Category: Bug report » Feature request
Status: Closed (works as designed) » Active

Hi @jcnventura.

Sorry for the trouble with the patch. The guys are new to the community and we are trying to help them get used to the flow. Unfortunately, I was not able to properly review the uploaded patch, which caused all this confusion. I'm sorry for that.

Anyway, we are proposing the addition of a new field to the configuration for uploading a proper icon for the print functionality. This could also be implemented for the other 2 icons but we would like to have an opinion about this first. We thought that it would be too much to implement a theme_print_ui_format_link() just to change this link because we could create a field instead and simply upload the icon. Still, we want the opinion/review from someone else. We will upload the new patch soon and wait for a review. I'm also changing this to a feature request.

Thank you.

mariosr’s picture

Uploading the new proposed patch.
Please review.

mariosr’s picture

Status: Active » Needs review
jcnventura’s picture

Status: Needs review » Closed (works as designed)

If you truly want to change the icon you have a few choices:
1. Write your own version of theme_print_ui_format_link()
2. Enable the text-only version, and use CSS to insert your own logo.

I don't believe that module should support uploading your own icons.. That's the realm of custom themes, and the module already provides more than adequate support for those.

Obrigado a todos ai na CI&T, mas espero que compreendam que este patch não será aplicado.