http://getbootstrap.com/getting-started/#migration

.btn must also use .btn-default to get the "default" button.

Files: 
CommentFileSizeAuthor
#9 bootstrap-3-add_default_class-2073237-9.patch1.36 KBheylookalive
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#7 bootstrap-add-default-class-button-and-var-codingstd.patch1.4 KBoriol_e9g
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch bootstrap-add-default-class-button-and-var-codingstd.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#5 bootstrap-add-default-class-button-and-var-2073237-5.patch1.39 KBvalkum
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#3 bootstrap-add-default-class-button-2073237-3.patch1.19 KBel7cosmos
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#1 bootstrap-add-default-class-button-2073237-1.patch807 bytesel7cosmos
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new807 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Status:Needs review» Needs work

+++ b/includes/theme.inc
@@ -126,7 +126,7 @@ function theme_bootstrap_links($variables) {
-  $type_class = '';
+  $type_class = 'btn-default';
...
   $variables['attributes']['class'][] = 'btn-group';

Setting $type_class here will potentially get overwritten on ln 135.

You should add similarly like the btn-group.

Status:Needs work» Needs review
StatusFileSize
new1.19 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Yeah it meant to be overridden by other modifier.

But problem is, I get repetitive btn-default class in theme_button since it loops each array element to check button value, so I decide to assign default class to be overridden as the above case.

Status:Needs review» Needs work

I'm just not too thrilled with this approach TBCH. It needs work. Feel free to completely refactor it if needed (instead of trying to work within the confines of the existing code).

Status:Needs work» Needs review
StatusFileSize
new1.39 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Hey there, i took patch #3 and restructured the code a little bit. Also i added a var for theme_button called type similar to btn_dropdown

It works so far.

Works for me too. Looks good.

StatusFileSize
new1.4 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch bootstrap-add-default-class-button-and-var-codingstd.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Coding standards

Status:Needs review» Needs work

The last submitted patch, bootstrap-add-default-class-button-and-var-codingstd.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.36 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

How about this, if we don't have any decent classes on a button at theme_button stage, throw in the default. Much simpler than other methods.

Also have got the bootstrap dropdown change present in other patches, I don't have eyes on that as an issue though so am not sure if that's correct or not.

Status:Needs review» Needs work
  1. +++ b/includes/form.inc
    @@ -260,6 +260,22 @@ function bootstrap_button($variables) {
       if (!empty($element['#attributes']['disabled'])) {
         $element['#attributes']['class'][] = 'form-button-disabled';

    Is this still the correct class?

  2. +++ b/includes/theme.inc
    @@ -134,6 +134,9 @@ function theme_bootstrap_btn_dropdown($variables) {
       if (isset($variables['type'])) {
         $type_class = ' btn-'. $variables['type'];
       }
    +  else {
    +    $type_class = ' btn-default';
    +  }

    I'd really not have to do this. Is there a reason why we can't just throw out all the old crap?

.btn must also use .btn-default to get the "default" button.

http://getbootstrap.com/getting-started/#migration

.btn .btn-default

@mark disabled in bS3 works off of the button element disabled attribute which by the looks of that code should be fine, but will have an extra class on it whilst not needed could be helpful to people for theming.

On 2, can't say that I use this but for backwards compatibility should stick with it I guess (as a maintainer you can make the call to bin it, i have no preference personally), but as said I haven't paid any attention to this chunk of functionality/theming. The change makes logical sense in providing the default class. @morningtime, will need to check how this works but yep should have the ".btn" class on it as per docs.

but will have an extra class on it whilst not needed could be helpful to people for theming.

.btn[disabled] will work just fine. Having classes like this is superfluous. I know it wasn't added in this patch, but I'd like to just remove it here. My goal with the migration from 2.x to 3.x wasn't just a simple conversion, but also cleanup of the cruft that has been added over time through the various project merges.

For #10.2, I say we either remove this or change it from .btn-$type to .form-$type. I don't want to confuse the .btn prefix with Drupal specifics.

Isn't the $variable type param meant to be set by someone who's using this hook? To set which style the dropdown button is shown?

@valkum, no the dropdown buttons have different classes and markup altogether (see: http://getbootstrap.com/components/#btn-dropdowns). The $variables['#type'] is the type of element in the FAPI. It has nothing to do with Bootstrap.

@heylookalive in #12, in response to backwards compatibility: The migration from 2.x to 3.x is not meant to be "compatible" there is too much that has changed. I'm ok with "breaking" BC here.

It is 'type' not '#type'
From our hook_theme definition

<?php
...
'bootstrap_btn_dropdown' => array(
     
'variables' => array(
       
'links' => array(),
       
'attributes' => array(),
       
'type' => NULL
     
),
    ),
...
?>

I thought someone who's using the bootstrap_btn_dropdown theme in his subtheme or module can define which option is set as class for the buttin (primary, alert, etc.)

Ah... good catch. I forget about that theme hook sometimes. Thanks! Yeah, I guess leave it in for now, make sure it works though. Some 'type' classes might need updating.

Status:Needs work» Fixed

Thanks @heylookalive!

Committed 24dd443 to 7.x-3.x.
Committed d4973fc to 7.x-3.x.

Version:7.x-3.x-dev» 7.x-3.0-beta1

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.