Fix notification drawer accordion panel sizing #548
Conversation
| @@ -78,24 +85,21 @@ angular.module('patternfly.utils').directive('pfFixedAccordion', function ($wind | |||
|
|
|||
| if (angular.isDefined($scope.scrollSelector)) { | |||
| scroller = angular.element(collapsePanel[0].querySelector($scope.scrollSelector)); | |||
| if (scroller.length === 1) { | |||
| if (scroller.length > 0) { | |||
| scrollElement = angular.element(scroller[0]); | |||
Update sizes when elements in the notification drawer are updated. Update collapse children to not have a intermediate parent. Fix for example notification drawer body html indentation error. Fixes patternfly#545
jeff-phillips-18
force-pushed
the
notifications
branch
from
c32c32a to
7646c4e
Compare
| @@ -51,6 +51,12 @@ angular.module('patternfly.notification').component('pfNotificationDrawer', { | |||
| }); | |||
| }; | |||
|
|
|||
| var updateAccordionSizing = function () { | |||
| $timeout(function () { | |||
There was a problem hiding this comment.
Any concerns about performance here? if $onChanges is triggered frequently, do you want to be triggering resize often, or would a debounce/throttle be in order? I'm also curious about 100, vs (0 or any other small #) if its arbitrary or standardized across patternfly angular for this kind of thing.
There was a problem hiding this comment.
I don't believe onChanges will get call that frequently. Typically most of these won't change, but should they, the size needs to be recalculated. Also, its only done if the draw is shown and one of these values change.
We've typically used 100 just to get past any follow up changes and get thru a digest cycle or two. Main idea is to get past the current digest cycle anyhow.
There was a problem hiding this comment.
Just a consistency nit for truthy check. @benjaminapetersen comment about $timeout vs $onChanges looks like it warrants a look if you haven't already.
| @@ -19,16 +19,15 @@ <h4 class="panel-title"> | |||
| <span class="panel-counter" ng-include src="$ctrl.subheadingInclude"></span> | |||
| </div> | |||
| <div class="panel-collapse collapse" ng-class="{in: notificationGroup.open || $ctrl.notificationGroups.length === 1}"> | |||
There was a problem hiding this comment.
Truthy ~ $ctrl.notificationGroups.length vs $ctrl.notificationGroups.length === 1
There was a problem hiding this comment.
But it must be exactly 1.
There was a problem hiding this comment.
Right, this is the single panel option. Sorry, just saw the truthy "huh" :)
|
@jeff-phillips-18 I've built this & pulled it into the console to test. Works much better, big improvement, thx! |
Description
Update sizes when elements in the notification drawer are updated.
Update collapse children to not have a intermediate parent.
Fix for example notification drawer body html indentation error.
Fixes #545
@benjaminapetersen