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.
As per @kslonka's previous patch, this is a smaller patch which removes the unused versions and updates the CDN location
Comment | File | Size | Author |
---|---|---|---|
#15 | #2071683-1(cdn-none).png | 71.37 KB | nithinkolekar |
#10 | bootstrap-update-cdn-2071683-10.patch | 6.44 KB | heylookalive |
#8 | bootstrap-update-cdn-2071683-8.patch | 6.47 KB | heylookalive |
#5 | bootstrap-update-cdn-2071683-5.patch | 6.35 KB | heylookalive |
#3 | bootstrap-3-update-cdn-2071683-3.patch | 2.09 KB | heylookalive |
Comments
Comment #1
olamaekle CreditAttribution: olamaekle commentedShouldn't you change the 'cdn_bootstrap_version' setting in the .info file to 3.0.0?
Comment #2
markhalliwellI would also like to take this opportunity to change and remove unnecessary variable names too. In reality it should be just a simple:
bootstrap_cdn
This variable would be "enabled" if that variable has a version in it and "disabled" if the variable is empty.
We should also use the #empty_option property in the select field.
Comment #3
heylookalive CreditAttribution: heylookalive commentedUpdated patch changing the setting in the .info.
I'd say for the sake of one extra variable for the CDN it's not worth removing the checkbox considering the clarity it brings, but if there's a strong feeling about it can roll a patch removing it.
Comment #4
markhalliwell@heylookalive, yes. I would like to turn this into a select drop down with "- None -" at the top of the selection list. There really is no point in having two variables to set (ie: one to "enable" it and one to select the version). There should just be one variable that has the version or is empty (thus disabling the CDN). I really appreciate you stepping up with this btw :) Once these changes have been made, I'll commit.
Comment #5
heylookalive CreditAttribution: heylookalive commentedNo worries :)
Here we go, one var, have reworked that form slightly and gone for a more explicit empty option label, reworked a few bits of text and done a few coding standards bits.
Comment #6
heylookalive CreditAttribution: heylookalive commentedComment #7
markhalliwellNow that I think about this and see the code, we might as well just group all these settings into something like a "Bootstrap settings" fieldset with the key value of
$form['bootstrap']
.It's really kind of bad UX to duplicate both a "Theme CDN settings" title and then below show it again for the actual select field. We could move the
#description
down to the select field.I would just label this as "BootstrapCDN version"
Let's drop the "v" prefix and just do:
I'd really rather this just be "Disabled"
Comment #8
heylookalive CreditAttribution: heylookalive commentedHave made the above changes, I think the form item label for the CDN dropdown has become less explicit. It's clearly explained in the readme and field description but people tend to not look at those things...
Comment #9
markhalliwellIs there a reason for this change? Also, there is a space at EOL.
Let's just title this "BootstrapCDN". The description explains the title.
How about we change this to "Use the BootstrapCDN (content distribution network) to serve the Bootstrap framework files. Enabling this will prevent this theme from trying to load local files."
edit: keep the warning after too though.
Comment #10
heylookalive CreditAttribution: heylookalive commentedThe above changes made, I'm happy with this.
On point one that's a change for the switch in variable names, and was meant to be an empty value which I've replaced with '';
Comment #11
markhalliwell@heylookalive thanks!
Committed: 16dd99f
Comment #12
heylookalive CreditAttribution: heylookalive commentedYesss! :)
Comment #14
markhalliwellComment #15
nithinkolekar CreditAttribution: nithinkolekar commented@markcarver
If CDN setting is set to "-none-" and bootswatch css,js files are added locally to subtheme , should it be mandatory to have core bootstrap css file too?
Bootswatch css file ~145kb which is same as core css file and I thought it has all the code as core but with style modification. Is that correct?
main issue: with the above configuration admin menu is overlapped by nav bar. If cdn value is set to 3.3.5 or any other value everything works fine.(~300kb in total for two files).
Comment #16
nithinkolekar CreditAttribution: nithinkolekar commentedupdate on #15
/sites/all/themes/bootstrap/css/3.3.5/overrides.min.css?p07ytg is not loading when CDN set to none and I tested it by copying css code and applied in firefox style editor then nav bar shifts down properly.
Comment #17
markhalliwellNo. The compiled overrides CSS only shows up if you use the CDN (because we can't control what the CDN serves).
The intended purpose of doing any "local" files is to utilize (and change) the source files (LESS or SASS) to create a custom theme. The LESS and SASS starterkits come with the overrides source files that you can customize and compile into your sub-theme.
If you're using a "local" compiled copy of Bootstrap/Bootswatch, then you'd be better off just using the CDN in the first place. If you cannot, for whatever reason, then you will need to copy the appropriate compiled overrides CSS from the base theme into your sub-theme and add it to the
*.libraries.yml
file.