Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Commit 7fdf9da

Browse files
Splaktarmmalerba
authored andcommitted
fix(tabs): md-center-tabs causes in high CPU usage (#11375)
<!-- Filling out this template is required! Do not delete it when submitting a Pull Request! Without this information, your Pull Request may be auto-closed. --> ## PR Checklist Please check that your PR fulfills the following requirements: - [x] The commit message follows [our guidelines](https://github.com/angular/material/blob/master/.github/CONTRIBUTING.md#-commit-message-format) - [ ] Tests for the changes have been added or this is not a bug fix / enhancement - [x] Docs have been added, updated, or were not required ## PR Type What kind of change does this PR introduce? <!-- Please check the one that applies to this PR using "x". --> ``` [x] Bugfix [ ] Enhancement [ ] Documentation content changes [ ] Code style update (formatting, local variables) [ ] Refactoring (no functional changes, no api changes) [ ] Build related changes [ ] CI related changes [ ] Infrastructure changes [ ] Other... Please describe: ``` ## What is the current behavior? `md-center-tabs` causes constant 30-50% CPU usage due to indefinite execution of `updateInkBarStyles()`; <!-- Please describe the current behavior that you are modifying and link to one or more relevant issues. --> Issue Number: Fixes #9690. Fixes #6375. ## What is the new behavior? - make updateInkBarStyles() a noop if md-no-ink-bar is set - remove md-no-ink docs as the feature does not exist - clean up md-tab docs - add JSDoc and improve comments in TabsController - add TODOs for min/max widths that no longer match the latest MD spec - update URLs to MD spec ## Does this PR introduce a breaking change? ``` [ ] Yes [x] No ``` <!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. --> <!-- Note that breaking changes are highly unlikely to get merged to master unless the validation is clear and the use case is critical. --> ## Other information
1 parent 998199f commit 7fdf9da

File tree

3 files changed

+123
-69
lines changed

3 files changed

+123
-69
lines changed

src/components/tabs/js/tabDirective.js

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,43 +6,50 @@
66
* @restrict E
77
*
88
* @description
9-
* The `<md-tab>` is a nested directive used within `<md-tabs>` to specify a tab with a **label** and optional *view content*.
9+
* The `<md-tab>` is a nested directive used within `<md-tabs>` to specify a tab with a **label**
10+
* and optional *view content*.
1011
*
11-
* If the `label` attribute is not specified, then an optional `<md-tab-label>` tag can be used to specify more
12-
* complex tab header markup. If neither the **label** nor the **md-tab-label** are specified, then the nested
13-
* markup of the `<md-tab>` is used as the tab header markup.
12+
* If the `label` attribute is not specified, then an optional `<md-tab-label>` tag can be used to
13+
* specify more complex tab header markup. If neither the **label** nor the **md-tab-label** are
14+
* specified, then the nested markup of the `<md-tab>` is used as the tab header markup.
1415
*
15-
* Please note that if you use `<md-tab-label>`, your content **MUST** be wrapped in the `<md-tab-body>` tag. This
16-
* is to define a clear separation between the tab content and the tab label.
16+
* Please note that if you use `<md-tab-label>`, your content **MUST** be wrapped in the
17+
* `<md-tab-body>` tag. This is to define a clear separation between the tab content and the tab
18+
* label.
1719
*
18-
* This container is used by the TabsController to show/hide the active tab's content view. This synchronization is
19-
* automatically managed by the internal TabsController whenever the tab selection changes. Selection changes can
20-
* be initiated via data binding changes, programmatic invocation, or user gestures.
20+
* This container is used by the TabsController to show/hide the active tab's content view. This
21+
* synchronization is automatically managed by the internal TabsController whenever the tab
22+
* selection changes. Selection changes can be initiated via data binding changes, programmatic
23+
* invocation, or user gestures.
2124
*
2225
* @param {string=} label Optional attribute to specify a simple string as the tab label
23-
* @param {boolean=} ng-disabled If present and expression evaluates to truthy, disabled tab selection.
24-
* @param {expression=} md-on-deselect Expression to be evaluated after the tab has been de-selected.
26+
* @param {boolean=} ng-disabled If present and expression evaluates to truthy, disabled tab
27+
* selection.
28+
* @param {expression=} md-on-deselect Expression to be evaluated after the tab has been
29+
* de-selected.
2530
* @param {expression=} md-on-select Expression to be evaluated after the tab has been selected.
26-
* @param {boolean=} md-active When true, sets the active tab. Note: There can only be one active tab at a time.
31+
* @param {boolean=} md-active When true, sets the active tab. Note: There can only be one active
32+
* tab at a time.
2733
*
2834
*
2935
* @usage
3036
*
3137
* <hljs lang="html">
32-
* <md-tab label="" ng-disabled md-on-select="" md-on-deselect="" >
38+
* <md-tab label="My Tab" ng-disabled md-on-select="onSelect()" md-on-deselect="onDeselect()">
3339
* <h3>My Tab content</h3>
3440
* </md-tab>
3541
*
36-
* <md-tab >
42+
* <md-tab>
3743
* <md-tab-label>
38-
* <h3>My Tab content</h3>
44+
* <h3>My Tab</h3>
3945
* </md-tab-label>
4046
* <md-tab-body>
4147
* <p>
42-
* Sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium,
43-
* totam rem aperiam, eaque ipsa quae ab illo inventore veritatis et quasi architecto beatae vitae
44-
* dicta sunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit aspernatur aut odit aut fugit,
45-
* sed quia consequuntur magni dolores eos qui ratione voluptatem sequi nesciunt.
48+
* Sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque
49+
* laudantium, totam rem aperiam, eaque ipsa quae ab illo inventore veritatis et quasi
50+
* architecto beatae vitae dicta sunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit
51+
* aspernatur aut odit aut fugit, sed quia consequuntur magni dolores eos qui ratione
52+
* voluptatem sequi nesciunt.
4653
* </p>
4754
* </md-tab-body>
4855
* </md-tab>

src/components/tabs/js/tabsController.js

Lines changed: 97 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ angular
66
* @ngInject
77
*/
88
function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipple, $mdUtil,
9-
$animateCss, $attrs, $compile, $mdTheming, $mdInteraction,
9+
$animateCss, $attrs, $compile, $mdTheming, $mdInteraction, $timeout,
1010
MdTabsPaginationService) {
1111
// define private properties
1212
var ctrl = this,
@@ -44,7 +44,8 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
4444

4545
/**
4646
* AngularJS Lifecycle hook for newer AngularJS versions.
47-
* Bindings are not guaranteed to have been assigned in the controller, but they are in the $onInit hook.
47+
* Bindings are not guaranteed to have been assigned in the controller, but they are in the
48+
* $onInit hook.
4849
*/
4950
function $onInit() {
5051
// Define one-way bindings
@@ -141,10 +142,10 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
141142
}
142143

143144
/**
144-
* Defines boolean attributes with default value set to true. (ie. md-stretch-tabs with no value
145-
* will be treated as being truthy)
146-
* @param key
147-
* @param handler
145+
* Defines boolean attributes with default value set to true. I.e. md-stretch-tabs with no value
146+
* will be treated as being truthy.
147+
* @param {string} key
148+
* @param {Function} handler
148149
*/
149150
function defineBooleanAttribute (key, handler) {
150151
var attr = $attrs.$normalize('md-' + key);
@@ -167,19 +168,25 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
167168
// Change handlers
168169

169170
/**
170-
* Toggles stretch tabs class and updates inkbar when tab stretching changes
171-
* @param stretchTabs
171+
* Toggles stretch tabs class and updates inkbar when tab stretching changes.
172172
*/
173-
function handleStretchTabs (stretchTabs) {
173+
function handleStretchTabs () {
174174
var elements = getElements();
175175
angular.element(elements.wrapper).toggleClass('md-stretch-tabs', shouldStretchTabs());
176176
updateInkBarStyles();
177177
}
178178

179-
function handleCenterTabs (newValue) {
179+
/**
180+
* Update the value of ctrl.shouldCenterTabs.
181+
*/
182+
function handleCenterTabs () {
180183
ctrl.shouldCenterTabs = shouldCenterTabs();
181184
}
182185

186+
/**
187+
* @param {number} newWidth new max tab width in pixels
188+
* @param {number} oldWidth previous max tab width in pixels
189+
*/
183190
function handleMaxTabWidth (newWidth, oldWidth) {
184191
if (newWidth !== oldWidth) {
185192
var elements = getElements();
@@ -246,8 +253,8 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
246253

247254
/**
248255
* Update the UI whenever the selected index changes. Calls user-defined select/deselect methods.
249-
* @param newValue
250-
* @param oldValue
256+
* @param {number} newValue selected index's new value
257+
* @param {number} oldValue selected index's previous value
251258
*/
252259
function handleSelectedIndexChange (newValue, oldValue) {
253260
if (newValue === oldValue) return;
@@ -295,7 +302,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
295302

296303
/**
297304
* Handle user keyboard interactions
298-
* @param event
305+
* @param {KeyboardEvent} event keydown event
299306
*/
300307
function keydown (event) {
301308
switch (event.keyCode) {
@@ -384,21 +391,27 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
384391
});
385392
}
386393

394+
/**
395+
* Hides or shows the tabs ink bar.
396+
* @param {boolean} hide A Boolean (not just truthy/falsy) value to determine whether the class
397+
* should be added or removed.
398+
*/
387399
function handleInkBar (hide) {
388400
angular.element(getElements().inkBar).toggleClass('ng-hide', hide);
389401
}
390402

391403
/**
392-
* Toggle dynamic height class when value changes
393-
* @param value
404+
* Enables or disables tabs dynamic height.
405+
* @param {boolean} value A Boolean (not just truthy/falsy) value to determine whether the class
406+
* should be added or removed.
394407
*/
395408
function handleDynamicHeight (value) {
396409
$element.toggleClass('md-dynamic-height', value);
397410
}
398411

399412
/**
400413
* Remove a tab from the data and select the nearest valid tab.
401-
* @param tabData
414+
* @param {Object} tabData tab to remove
402415
*/
403416
function removeTab (tabData) {
404417
if (destroyed) return;
@@ -419,8 +432,8 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
419432

420433
/**
421434
* Create an entry in the tabs array for a new tab at the specified index.
422-
* @param tabData
423-
* @param index
435+
* @param {Object} tabData tab to insert
436+
* @param {number} index location to insert the new tab
424437
* @returns {*}
425438
*/
426439
function insertTab (tabData, index) {
@@ -583,9 +596,9 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
583596
/**
584597
* Defines a property using a getter and setter in order to trigger a change handler without
585598
* using `$watch` to observe changes.
586-
* @param key
587-
* @param handler
588-
* @param value
599+
* @param {PropertyKey} key
600+
* @param {Function} handler
601+
* @param {any} value
589602
*/
590603
function defineProperty (key, handler, value) {
591604
Object.defineProperty(ctrl, key, {
@@ -608,12 +621,16 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
608621

609622
/**
610623
* Calculates the width of the pagination wrapper by summing the widths of the dummy tabs.
611-
* @returns {number}
624+
* @returns {number} the width of the pagination wrapper in pixels
612625
*/
613626
function calcPagingWidth () {
614627
return calcTabsWidth(getElements().tabs);
615628
}
616629

630+
/**
631+
* @param {Array<HTMLElement>} tabs tab item elements for use in computing total width
632+
* @returns {number} the width of the tabs in the specified array in pixels
633+
*/
617634
function calcTabsWidth(tabs) {
618635
var width = 0;
619636

@@ -628,25 +645,35 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
628645
return Math.ceil(width);
629646
}
630647

631-
function getMaxTabWidth () {
648+
/**
649+
* @returns {number} either the max width as constrained by the container or the max width from an
650+
* old version of the Material Design spec.
651+
* TODO update max tab width to equal the spec in 1.2.
652+
*/
653+
function getMaxTabWidth() {
632654
var elements = getElements(),
633-
containerWidth = elements.canvas.clientWidth,
655+
containerWidth = elements.canvas.clientWidth,
634656

635-
// See https://material.google.com/components/tabs.html#tabs-specs
636-
specMax = 264;
657+
// See https://material.io/design/components/tabs.html#spec which has been updated to 360px.
658+
specMax = 264;
637659

638660
// Do the spec maximum, or the canvas width; whichever is *smaller* (tabs larger than the canvas
639661
// width can break the pagination) but not less than 0
640662
return Math.max(0, Math.min(containerWidth - 1, specMax));
641663
}
642664

665+
/**
666+
* @returns {number} the min width from an old version of the Material Design spec. This returns
667+
* a larger min width if the container width is larger than 600px.
668+
* TODO update min tab width to equal the spec in 1.2.
669+
*/
643670
function getMinTabWidth() {
644671
var elements = getElements(),
645-
containerWidth = elements.canvas.clientWidth,
646-
xsBreakpoint = 600,
672+
containerWidth = elements.canvas.clientWidth,
673+
xsBreakpoint = 600,
647674

648-
// See https://material.google.com/components/tabs.html#tabs-specs
649-
specMin = containerWidth > xsBreakpoint ? 160 : 72;
675+
// See https://material.io/design/components/tabs.html#spec which has been updated to 90px.
676+
specMin = containerWidth > xsBreakpoint ? 160 : 72;
650677

651678
// Do the spec minimum, or the canvas width; whichever is *smaller* (tabs larger than the canvas
652679
// width can break the pagination) but not less than 0
@@ -668,8 +695,9 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
668695
}
669696

670697
/**
671-
* This moves the selected or focus index left or right. This is used by the keydown handler.
672-
* @param inc
698+
* This moves the selected or focus index left or right. This is used by the keydown handler.
699+
* @param {number} inc amount to increment
700+
* @param {boolean} focus true to increment the focus index, false to increment the selected index
673701
*/
674702
function incrementIndex (inc, focus) {
675703
var newIndex,
@@ -687,7 +715,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
687715
}
688716

689717
/**
690-
* This is used to forward focus to tab container elements. This method is necessary to avoid
718+
* This is used to forward focus to tab container elements. This method is necessary to avoid
691719
* animation issues when attempting to focus an item that is out of view.
692720
*/
693721
function redirectFocus () {
@@ -697,6 +725,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
697725

698726
/**
699727
* Forces the pagination to move the focused tab into view.
728+
* @param {number} index of tab to have its offset adjusted
700729
*/
701730
function adjustOffset (index) {
702731
var elements = getElements();
@@ -728,7 +757,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
728757
}
729758

730759
/**
731-
* Iterates through all queued functions and clears the queue. This is used for functions that
760+
* Iterates through all queued functions and clears the queue. This is used for functions that
732761
* are called before the UI is ready, such as size calculations.
733762
*/
734763
function processQueue () {
@@ -740,9 +769,10 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
740769
* Determines if the tab content area is needed.
741770
*/
742771
function updateHasContent () {
743-
var hasContent = false;
772+
var hasContent = false;
773+
var i;
744774

745-
for (var i = 0; i < ctrl.tabs.length; i++) {
775+
for (i = 0; i < ctrl.tabs.length; i++) {
746776
if (ctrl.tabs[i].hasContent) {
747777
hasContent = true;
748778
break;
@@ -784,7 +814,9 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
784814
currentHeight -= tabsHeight;
785815
newHeight -= tabsHeight;
786816
// Need to include bottom border in these calculations
787-
if ($element.attr('md-border-bottom') !== undefined) ++currentHeight;
817+
if ($element.attr('md-border-bottom') !== undefined) {
818+
++currentHeight;
819+
}
788820
}
789821

790822
// Lock during animation so the user can't change tabs
@@ -825,20 +857,33 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
825857

826858
/**
827859
* Repositions the ink bar to the selected tab.
828-
* @returns {*}
829-
*/
830-
function updateInkBarStyles () {
860+
* Parameters are used when calling itself recursively when md-center-tabs is used as we need to
861+
* run two passes to properly center the tabs. These parameters ensure that we only run two passes
862+
* and that we don't run indefinitely.
863+
* @param {number=} previousTotalWidth previous width of pagination wrapper
864+
* @param {number=} previousWidthOfTabItems previous width of all tab items
865+
*/
866+
function updateInkBarStyles (previousTotalWidth, previousWidthOfTabItems) {
867+
if (ctrl.noInkBar) {
868+
return;
869+
}
831870
var elements = getElements();
832871

833872
if (!elements.tabs[ ctrl.selectedIndex ]) {
834873
angular.element(elements.inkBar).css({ left: 'auto', right: 'auto' });
835874
return;
836875
}
837876

838-
if (!ctrl.tabs.length) return queue.push(ctrl.updateInkBarStyles);
839-
// if the element is not visible, we will not be able to calculate sizes until it is
840-
// we should treat that as a resize event rather than just updating the ink bar
841-
if (!$element.prop('offsetParent')) return handleResizeWhenVisible();
877+
if (!ctrl.tabs.length) {
878+
queue.push(ctrl.updateInkBarStyles);
879+
return;
880+
}
881+
// If the element is not visible, we will not be able to calculate sizes until it becomes
882+
// visible. We should treat that as a resize event rather than just updating the ink bar.
883+
if (!$element.prop('offsetParent')) {
884+
handleResizeWhenVisible();
885+
return;
886+
}
842887

843888
var index = ctrl.selectedIndex,
844889
totalWidth = elements.paging.offsetWidth,
@@ -847,11 +892,14 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
847892
right = totalWidth - left - tab.offsetWidth;
848893

849894
if (ctrl.shouldCenterTabs) {
850-
// We need to use the same calculate process as in the pagination wrapper, to avoid rounding deviations.
851-
var tabWidth = calcTabsWidth(elements.tabs);
852-
853-
if (totalWidth > tabWidth) {
854-
$mdUtil.nextTick(updateInkBarStyles, false);
895+
// We need to use the same calculate process as in the pagination wrapper, to avoid rounding
896+
// deviations.
897+
var totalWidthOfTabItems = calcTabsWidth(elements.tabs);
898+
899+
if (totalWidth > totalWidthOfTabItems &&
900+
previousTotalWidth !== totalWidth &&
901+
previousWidthOfTabItems !== totalWidthOfTabItems) {
902+
$timeout(updateInkBarStyles, 0, true, totalWidth, totalWidthOfTabItems);
855903
}
856904
}
857905
updateInkBarClassName();

0 commit comments

Comments
 (0)