Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typo in breadcrumb_classes setting #217

Open
wants to merge 2 commits into
base: 7.x-2.x
Choose a base branch
from

Conversation

lennart
Copy link

@lennart lennart commented Oct 18, 2013

This pull-request will sanitize the option value accepted by the theme setting breadcrumb_classes by removing the trailing space.

I know this is not backwards compatibly, but please think about adjusting your last change to a more sane option-value. Maybe one could run a migration on updating the theme, but I am not that much into maintaining Drupal Modules. If you can lead the way, I am happy to integrate a simple migration that fixes existing database values as well.

The two commits fix the theme settings admin pane (theme-settings.php) and the interpretation of the value inside template.php

Best regards and keep up the good work,

Lennart

@ghost ghost assigned hyperboy Oct 28, 2013
@meganem
Copy link
Contributor

meganem commented Oct 28, 2013

Thanks @lennart ! @hyperboy can you take a look at this pull request when you have a chance?

@meganem
Copy link
Contributor

meganem commented Mar 12, 2014

Risk is that a page template is checking using an if/then statement using the existing value and it would not recognize the new value.

We would need to make sure to support backwards compatibility on this, so probably not merging right now.

@meganem meganem modified the milestones: Icebox, 7.x-2.1 Mar 12, 2014
@meganem
Copy link
Contributor

meganem commented Jan 29, 2015

Because of our hosting ecosystem which does automatic theme updates, we have to have a backwards compatible solution. :/ Tabled for a potential future release..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants