Success

User tests: Successful: Unsuccessful:

avatar anibalsanchez
anibalsanchez
20 May 2013

Hi,

These are the files to update to latest Bootstrap 2.3.2.

I've added a Makefile and the required files to rebuild a Boostratp distribution.

Also, I've added new files to identify Joomla modification of the original Bootstrap Js and Less files. Next time, it's going to be easier to identify the modifications:

  1. div.modal (instead of .modal)
    *** modals.joomla.less
    *** responsive-767px-max.joomla.less
  2. hideme x hide ($.Event('hideme'))
    *** bootstrap-collapse-joomla.js
    *** bootstrap-tooltip-joomla.js
  3. Click on parent menu
    *** bootstrap-dropdown-joomla.js

The modifications have the preffix /* >>> JUI >>> / and / <<< JUI <<< */

Thanks,
Anibal

avatar anibalsanchez anibalsanchez - open - 20 May 2013
avatar wilsonge
wilsonge - comment - 20 May 2013

Whilst we're moving across to the integrated tracker - could you please create a tracker item for this here please? http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemBrowse&tracker_id=8103 :)

avatar mbabker
mbabker - comment - 20 May 2013

While this will update the main files to Bootstrap 2.3.2, what about updating the templates? Both Isis and Protostar (default front and back end templates) are built on Bootstrap and as such should be updated as well (CSS recompiled using our compiler at build/generatecss.php) and any fixes in the markup and JHtml helpers applied.

Also, why did you add the makefile in here? I don't think we need to include the makefile in the CMS as we distribute pretty much the stock Bootstrap stuff with only modifications needed to work in our environment (mostly to do with our use of Mootools still).

Lastly, why have you added separate JS files for every function? IIRC, the original idea was to only use the main bootstrap(.min).js file for everything to keep things as lightweight as possible.

avatar anibalsanchez
anibalsanchez - comment - 20 May 2013

Hi Michael,

About the Isis and Protostar, I haven't recompiled them. I haven't detected any need to modify markup or JHtml helpers.

As the modified Less files are being included now, I added the Makefile, Less files, and Js joomla files to identify the changes, and simplify the compilation.

If the filename is the same, it's a common mistake to assume they are the same than the original Bootstrap distribution (In fact, that was my first try to solve a Bootstrap bug, until I discovered the internal modifications).

Today, the changes are currently identified in the compiled bootstrap.js. I think it's better documented if you can re-create bootstrap.js and bootstrap.min.js in any time from modified source files. Eg, if someone wants to further customize dropdown-joomla.js

Thanks,
Anibal

avatar mbabker
mbabker - comment - 20 May 2013

If the templates' CSS files aren't recompiled, then there's honestly no reason to update the LESS and JavaScript files only. The templates should be recompiled with this change so that our Bug Squad can properly test these changes. Just modifying the JS and LESS files gives our folks nothing to test.

The CMS ships a solution for the 90% IMO. Most users are just going to install the CMS, install a template, and hope for the best. The 10% are the advanced users who will do the high end customizations and would truly need the makefile. Including it as is from Bootstrap also spells trouble as it is dependent on the file structure of the twitter/bootstrap repository. We have our own LESS compiler we use in the Git repo (not distributed with the installable CMS packages), so we don't need the makefile to do that. So, if the intention is to really have a makefile available to process the JavaScript files, my suggestion would be to put it in the build/ folder so that it's available for those who would need it but not distributed with the CMS.

We shouldn't distribute the fully compiled bootstrap.js file and each JS component file separately. That was a debate that was had last summer while working on integrating Bootstrap and I think most agreed the best solution was to just ship the fully compiled bootstrap.js and clearly mark any changes we had to make, keeping those changes to the bare minimum. Along the lines of the makefile argument, I would say if we really NEED to have the separate component JS files to put them in the build folder. This again makes the modified files available to those who would need them but doesn't add them into the CMS distribution. We could then clearly document our processes as it pertains to making changes to the various files and have a record somewhere of what was changed and why so that if a future version of Bootstrap makes those changes no longer necessary, we can drop our hack.

avatar anibalsanchez
anibalsanchez - comment - 20 May 2013

Ok, I'm going to resend you a new PR following these guidelines, moving the
source files to the build directory.

About the Isis and Protostar templates, the generatecss.php compiles the
bootstrap-extended.less and bootstrap-rtl.less.... but these files are not
included in the Bootstrap package. Do they have to be updated? From where?

Please, confirm me about this last subject to send you the new PR.

Thanks,
Anibal

avatar mbabker
mbabker - comment - 20 May 2013

bootstrap-extended.less is our own LESS file where we added some stuff. I think bootstrap-rtl.less is our's also and helps out with RTL language rendering.

avatar anibalsanchez
anibalsanchez - comment - 20 May 2013

So.... if they are Joomla native files, there's no way to update them from
the upstream.

I think the testing only have to validate if there's any impact from the
new bootstrap.min.js, and there's no need to re-generate the Isis and
Protostar templates.

If the templates have a bug, it has to be traced to the source of the
problem. If it's solved in Bootstrap newer version, or it was introduced in
the later customizations. It's not clear to me how the customized
template.css is generated, or if it's a full set of customized
Bootstrap-like classes.

To solve this PR, do I proceed to submit the new PR only with the Bootstrap
compiled files, and the source files moved to build?

Thanks,
Anibal

avatar mbabker
mbabker - comment - 20 May 2013

The templates compile their template.css files based on the LESS files specified in the template specific LESS files. Isis, Protostar, and Hathor each have their own LESS files that are compiled. If you use the generatecss.php script as a point of reference, it will tell you which LESS file is used to compile which CSS file for each template.

The templates should be in sync with the LESS files. It would be rather awkward to be using Bootstrap 2.1.0 CSS and Bootstrap 2.3.2 JS.

What we have in media/jui/less now should stay there (so all the Bootstrap LESS files and our customized stuff). The compiled bootstrap(.min).css and bootstrap(.min).js should remain in their current locations too. The makefile and the component JS files should move under the build folder (perhaps build/bootstrap).

If you just update the branch this PR is based from, this PR will automatically update with the changes you make.

avatar anibalsanchez
anibalsanchez - comment - 20 May 2013

Ok Cool, tomorrow I'm closing this PR and submitting the new one. At the
end, we are solving all Bootstrap-Javascript related bugs, improvements,
enabling the new media component, etc.

About the templates update, I see no easy way to update them. Protostar
template is a 120k css file. In a simple file comparison, there's a lot of
icons, colors and font changes. I hope it was generated with the online
Bootstrap tool (parameters?). If not, and it's a unique file; it has to be
fully recreated with the new version.

Thanks,
Anibal

avatar mbabker
mbabker - comment - 20 May 2013

All you have to do is execute the PHP script at build/generatecss.php. It'll update the CSS using the LESS files at media/jui/less.

avatar anibalsanchez
anibalsanchez - comment - 20 May 2013

The templates are not going to change. They don't use the new files.

        $templates = array(
            JPATH_ADMINISTRATOR . '/templates/isis/less/template.less' =>
JPATH_ADMINISTRATOR . '/templates/isis/css/template.css',
            JPATH_ADMINISTRATOR . '/templates/hathor/less/template.less' =>
JPATH_ADMINISTRATOR . '/templates/hathor/css/template.css',
            JPATH_ADMINISTRATOR . '/templates/hathor/less/colour_blue.less'
=> JPATH_ADMINISTRATOR . '/templates/hathor/css/colour_blue.css',
            JPATH_ADMINISTRATOR .
'/templates/hathor/less/colour_brown.less' => JPATH_ADMINISTRATOR .
'/templates/hathor/css/colour_brown.css',
            JPATH_ADMINISTRATOR .
'/templates/hathor/less/colour_standard.less' => JPATH_ADMINISTRATOR .
'/templates/hathor/css/colour_standard.css',
            JPATH_SITE . '/templates/protostar/less/template.less' =>
JPATH_SITE . '/templates/protostar/css/template.css',
*            // Below files are to recompile the default Bootstrap CSS files
            __DIR__ . '/less/bootstrap-extended.less' => JPATH_SITE .
'/media/jui/css/bootstrap-extended.css',
            __DIR__ . '/less/bootstrap-rtl.less' => JPATH_SITE .
'/media/jui/css/bootstrap-rtl.css'
        );

Thanks,

avatar mbabker
mbabker - comment - 20 May 2013

Directly, no, they don't. But if you look at the contents of https://github.com/joomla/joomla-cms/blob/master/administrator/templates/isis/less/template.less, you'll see that the Bootstrap LESS files are linked from there.

avatar anibalsanchez
anibalsanchez - comment - 20 May 2013

Ok, I see!, I'm also re-generating them.

Thanks,

avatar anibalsanchez anibalsanchez - close - 21 May 2013
avatar anibalsanchez
anibalsanchez - comment - 21 May 2013

Closing this PR, and submitting a new one with updated templates

avatar mhehm
mhehm - comment - 6 Jun 2013

Hi,
I already converted the bootstrap into right to left.
Might be useful for you.
http://mhehm.github.io/bootstrap/
https://github.com/mhehm/bootstrap

Add a Comment

Login with GitHub to post a comment