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 for adding CSS when CDN specified #347

Closed
wants to merge 1 commit into from

Conversation

dmbeer
Copy link
Contributor

@dmbeer dmbeer commented Mar 23, 2014

This fixed the issue of the CSS not being added when uising CDN.

@buildhive
Copy link

Michael Haitz » wicket-bootstrap #831 UNSTABLE
Looks like there's a problem with this pull request
(what's this?)

@@ -35,6 +36,9 @@ public void renderHead(final IBootstrapSettings settings, final IHeaderResponse
super.renderHead(settings, headerResponse);

// just includes all bootstrap resource references.
//adds the ccss header reference item. Makes sure that the CSS has been added if specified by CDN
final CssReferenceHeaderItem cssReferenceHeaderItem = CssReferenceHeaderItem.forReference(settings.getCssResourceReference(), "screen");
headerResponse.render(cssReferenceHeaderItem);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is done by BootstrapBaseBehavior.

public void renderHead(final IBootstrapSettings settings, final IHeaderResponse headerResponse) {
    final ITheme theme = settings.getActiveThemeProvider().getActiveTheme();

    theme.renderHead(headerResponse);
}

@l0rdn1kk0n
Copy link
Collaborator

the bug is located in Theme itself, i think we need to rework the theme handling.

How do you initialize bootstrap?

@martin-g
Copy link
Owner

It seems I have broken BootstrapTheme CDN support with:

commit af33ad5
Author: Martin Tzvetanov Grigorov [email protected]
Date: Mon Aug 19 11:31:01 2013 +0300

For some reason (that I cannot recall now) I have commented out the override of public Iterable<String> getCdnUrls()...

@martin-g
Copy link
Owner

But it seems to be fixed with 61276c3

@dmbeer
Copy link
Contributor Author

dmbeer commented Mar 24, 2014

@martin-g and @l0rdn1kk0n This seems to fail see #345 it adds a blank CSS at the top with no href value. If this is due to the default theme not being set then that is different.

The code change adds the CSS reference as per your commit above with this change.

l0rdn1kk0n added a commit that referenced this pull request Mar 25, 2014
@l0rdn1kk0n l0rdn1kk0n added this to the 0.9.x milestone Mar 25, 2014
@l0rdn1kk0n l0rdn1kk0n self-assigned this Mar 25, 2014
@martin-g
Copy link
Owner

We concluded that there is no issue in #345. Closing this issue as well.

@martin-g martin-g closed this Apr 25, 2014
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.

4 participants