Skip to content

Commit b185759

Browse files
committed
Fix overlay toolbar bug and streamline toolbar experience
1. Previously when you open a menu (e.g., delete resource) the actionbar would hide. Now this is fixed. 2. Started using official material style on toolbars. This means less our code.
1 parent 64afff1 commit b185759

File tree

6 files changed

+58
-66
lines changed

6 files changed

+58
-66
lines changed

src/app/frontend/chrome/chrome.html

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ <h2>
2828

2929
<kd-actionbar ng-if="ctrl.isActionbarVisible()"></kd-actionbar>
3030

31-
<md-content
32-
ng-class="{'kd-chrome-content': true, 'kd-chrome-has-actionbar': ctrl.isActionbarVisible()}">
31+
<md-content flex="auto" class="kd-app-content">
3332
<div ng-switch="ctrl.showLoadingSpinner" flex="auto" >
3433
<div ng-switch-when="true" class="kd-center-fixed">
3534
<md-progress-circular md-mode="indeterminate" md-diameter="96">

src/app/frontend/chrome/chrome.scss

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,25 +26,16 @@
2626

2727
.kd-toolbar {
2828
box-shadow: $whiteframe-shadow-1dp;
29-
height: $toolbar-height-size-base;
30-
position: fixed !important;
3129
}
3230

33-
.kd-chrome-content {
34-
padding-top: $toolbar-height-size-base;
35-
}
36-
37-
.kd-chrome-has-actionbar {
38-
padding-top: 2 * $toolbar-height-size-base;
39-
40-
@media only screen and (max-width: $layout-breakpoint-sm) {
41-
padding-top: $toolbar-height-size-base + $toolbar-height-size-base-sm;
42-
}
31+
chrome {
32+
display: block;
33+
overflow: hidden;
4334
}
4435

45-
body {
36+
body,
37+
.kd-app-content {
4638
background-color: $body;
47-
height: 100%;
4839
}
4940

5041
a {

src/app/frontend/common/components/actionbar/actionbar.scss

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,6 @@
1818
&.kd-actionbar {
1919
background: $content-background;
2020
color: $muted;
21-
height: $toolbar-height-size-base;
22-
top: $toolbar-height-size-base;
23-
24-
@media only screen and (min-width: 0) and (max-width: $layout-breakpoint-sm) {
25-
height: $toolbar-height-size-base-sm;
26-
}
2721
}
2822
}
2923

src/app/frontend/common/components/actionbar/actionbar_component.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,42 +17,42 @@
1717
*/
1818
export class ActionbarComponent {
1919
/**
20-
* @param {!angular.JQLite} $document
2120
* @param {!angular.JQLite} $element
2221
* @param {!angular.Scope} $scope
23-
* @param {!angular.$window} $window
2422
* @ngInject
2523
*/
26-
constructor($document, $element, $scope, $window) {
27-
/** @private {!angular.JQLite} */
28-
this.document_ = $document;
24+
constructor($element, $scope) {
2925
/** @private {!angular.JQLite}} */
3026
this.element_ = $element;
3127
/** @private {!angular.Scope} */
3228
this.scope_ = $scope;
33-
/** @private {!angular.$window} */
34-
this.window_ = $window;
3529
}
3630

3731
/**
3832
* @export
3933
*/
4034
$onInit() {
41-
this.computeScrollClass_();
35+
let closestContent = this.element_.parent().find('md-content');
36+
if (!closestContent || closestContent.length === 0) {
37+
throw new Error('Actionbar component requires sibling md-content element');
38+
}
4239

43-
let computeScrollClassCallback = () => { this.computeScrollClass_(); };
40+
this.computeScrollClass_(closestContent[0]); // Initialize scroll state at first.
4441

45-
this.document_.on('scroll', computeScrollClassCallback);
42+
let computeScrollClassCallback =
43+
(/** !Event */ e) => { this.computeScrollClass_(/** @type {!Element} */ (e.target)); };
44+
closestContent.on('scroll', computeScrollClassCallback);
4645
this.scope_.$on(
47-
'$destroy', () => { this.document_.off('scroll', computeScrollClassCallback); });
46+
'$destroy', () => { closestContent.off('scroll', computeScrollClassCallback); });
4847
}
4948

5049
/**
5150
* Computes scroll class based on scroll position and applies it to current element.
51+
* @param {!Element} mdContentElement
5252
* @private
5353
*/
54-
computeScrollClass_() {
55-
if (this.window_.scrollY > 0) {
54+
computeScrollClass_(mdContentElement) {
55+
if (mdContentElement.scrollTop > 0) {
5656
this.element_.removeClass('kd-actionbar-not-scrolled');
5757
} else {
5858
this.element_.addClass('kd-actionbar-not-scrolled');

src/app/frontend/index.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
experience.</p>
4545
<![endif]-->
4646

47-
<chrome>
47+
<chrome layout="column" layout-fill>
4848
<div ui-view> <!-- Application content is inserted here. --> </div>
4949
</chrome>
5050
<!-- build:js static/vendor.js -->

src/test/frontend/common/components/actionbar/actionbar_component_test.js

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,34 +17,42 @@ import actionbarCardModule from 'common/components/actionbar/actionbar_module';
1717
describe('Action bar', () => {
1818
beforeEach(() => angular.mock.module(actionbarCardModule.name));
1919

20-
it('should add shadow on scroll',
21-
angular.mock.inject(($rootScope, $window, $document, $compile) => {
22-
let elem = $compile('<kd-actionbar></kd-actionbar>')($rootScope);
23-
24-
// Start with no state.
25-
expect(elem[0].classList).not.toContain('kd-actionbar-not-scrolled');
26-
27-
$rootScope.$digest();
28-
// On initial load go with not scrolled state.
29-
expect(elem[0].classList).toContain('kd-actionbar-not-scrolled');
30-
31-
$window.scrollY = 70;
32-
$document.trigger('scroll');
33-
$rootScope.$digest();
34-
// Now it is scrolled
35-
expect(elem[0].classList).not.toContain('kd-actionbar-not-scrolled');
36-
37-
$window.scrollY = 0;
38-
$document.trigger('scroll');
39-
$rootScope.$digest();
40-
// Go back.
41-
expect(elem[0].classList).toContain('kd-actionbar-not-scrolled');
42-
43-
$rootScope.$destroy();
44-
$window.scrollY = 70;
45-
$document.trigger('scroll');
46-
$rootScope.$digest();
47-
// After $destroy - nothing happens.
48-
expect(elem[0].classList).toContain('kd-actionbar-not-scrolled');
49-
}));
20+
it('should add shadow on scroll', angular.mock.inject(($rootScope, $compile) => {
21+
let elem = $compile(`
22+
<div>
23+
<kd-actionbar></kd-actionbar>
24+
<md-content>foo</md-content>
25+
</div>`)($rootScope);
26+
27+
let mdContent = elem.find('md-content');
28+
let actionbar = elem.find('kd-actionbar');
29+
30+
// Start with no state.
31+
expect(actionbar[0].classList).not.toContain('kd-actionbar-not-scrolled');
32+
33+
$rootScope.$digest();
34+
let abCtrl = actionbar.controller('kdActionbar');
35+
// On initial load go with not scrolled state.
36+
expect(actionbar[0].classList).toContain('kd-actionbar-not-scrolled');
37+
38+
abCtrl.computeScrollClass_({scrollTop: 1});
39+
// Now it is scrolled
40+
expect(actionbar[0].classList).not.toContain('kd-actionbar-not-scrolled');
41+
42+
mdContent.trigger('scroll');
43+
$rootScope.$digest();
44+
// Go back.
45+
expect(actionbar[0].classList).toContain('kd-actionbar-not-scrolled');
46+
47+
$rootScope.$destroy();
48+
mdContent.trigger('scroll');
49+
$rootScope.$digest();
50+
// After $destroy - nothing happens.
51+
expect(actionbar[0].classList).toContain('kd-actionbar-not-scrolled');
52+
}));
53+
54+
it('should throw an error on no content', angular.mock.inject(($rootScope, $compile) => {
55+
$compile('<kd-actionbar></kd-actionbar>')($rootScope);
56+
expect(() => { $rootScope.$digest(); }).toThrow();
57+
}));
5058
});

0 commit comments

Comments
 (0)