Convert patternfly.form module directives to components. #375
Conversation
|
LGTM. Whjy are things moved into 'src/form/examples/', directives in 'examples' seem to be core directives (form-buttons, form-group, remain char count.), not examples. |
|
Only the ng-doc examples where moved there. I think this makes it easier to read the real component code and not have to look past all the ng-doc stuff. |
|
I know that you didn't write this originally but what are your thoughts on changing the form components to use normal BS3 form markup instead of |
|
Sorry, what is abnormal about form-horizontal? |
It's not that there is anything abnormal about it but I think the default should be the normal form and if we want a horizontal form we should have a different directive or different option for that. |
|
Probably a better way to go. Currently we default classes for the label and input components which really force the form-horizontal use. If we don't do that, the application could use either but will have to set the pfLabelClass and pfInputClass to use horizontal. |
jeff-phillips-18
force-pushed
the
branch-4.0-dev
branch
from
880e312 to
be38c16
Compare
Remove the default classes added to the label and input fields. This allows usage in any type of bootstrap form. Applications can simply add the classes via the pfLabelClass and pfInputClass bindings to use in a horizontal form. Added examples of both use cases.
jeff-phillips-18
force-pushed
the
branch-4.0-dev
branch
from
be38c16 to
2717b76
Compare
Exactly. This was originally copy/pasted from Katello where we were using horizontal and never updated to be more flexible. |
| /** | ||
| * @ngdoc directive | ||
| * @name patternfly.form.directive:pfFormGroup | ||
| * |
There was a problem hiding this comment.
restrict E should be in the header
| var ctrl = this; | ||
|
|
||
| if (ctrl.pfWorking === undefined) { | ||
| ctrl.pfWorking = false; |
There was a problem hiding this comment.
Generally any initialization code should be moved to $onInit
| transclude: true, | ||
| templateUrl: 'form/form-group/form-group.html', | ||
|
|
||
| controller: function ($scope, $element) { |
There was a problem hiding this comment.
$scope should be removed from the function
|
@dgutride Remaining characters count is not a component. Not sure how we would convert it. |
|
@dgutride Updated per your comments. |
| </pf-form-group> | ||
| </form> | ||
| <p>Horizontal Form</p> | ||
| <form class="form-horizontal"> |
There was a problem hiding this comment.
Should this be updated to reflect the discussed changes to pf-form-group (i.e. to remove the horizontal form specific classes)?
| <label for="{{ pfField }}" class="control-label {{ pfLabelClass }}">{{ pfLabel }}</label> | ||
| <div class="{{ pfInputClass }}"> | ||
| <div class="form-group" ng-class="{ 'has-error' : $ctrl.hasErrors() }"> | ||
| <label for="{{ $ctrl.pfField }}" class="control-label {{ $ctrl.pfLabelClass }}">{{ $ctrl.pfLabel }}</label> |
There was a problem hiding this comment.
Should this be updated to reflect the discussed changes to pf-form-group (i.e. to remove the horizontal form specific classes)?
There was a problem hiding this comment.
This is the example code. It shows both normal and horizontal forms:
|
Ah, missed the boat. I thought we were going to change the form groups to not be |
|
The are no longer .form-horizontal centric. |
👍 |
No description provided.