Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(top-app-bar): add default scroll behavior #2417

Merged
merged 26 commits into from
Apr 6, 2018
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3f3a9f9
feat(top-app-bar): Add default scroll behavior
williamernest Mar 8, 2018
1d8113f
feat(top-app-bar): Update for safari bug
williamernest Mar 8, 2018
3f325ec
feat(top-app-bar): Add resize handler
williamernest Mar 9, 2018
9783de2
WIP: Finish resize handling
williamernest Mar 12, 2018
283352d
feat(top-app-bar): Add comments
williamernest Mar 12, 2018
7fd0ad9
WIP: Add fixed-adjust classes for variants
williamernest Mar 13, 2018
18c8dda
WIP: Update scroll behavior. Add comments. Add closure annotations
williamernest Mar 14, 2018
6a97eac
feat(top-app-bar): Update for tests
williamernest Mar 16, 2018
3fb07bc
Merge branch 'master' into feat/top-app-bar/add-default-scroll-behavior
williamernest Mar 16, 2018
236eb64
WIP: Merge
williamernest Mar 17, 2018
4de6af2
feat(top-app-bar): Update for comments, merge with master
williamernest Mar 17, 2018
9f51fb5
Merge branch 'master' into feat/top-app-bar/add-default-scroll-behavior
williamernest Mar 17, 2018
4b9ce43
feat(top-app-bar): Update for comments
williamernest Mar 19, 2018
6544484
feat(top-app-bar): Update for comments
williamernest Mar 23, 2018
f7d3a46
feat(top-app-bar): Update for comments
williamernest Mar 27, 2018
e0004c0
feat(top-app-bar): Update top app bar scrolling logic
williamernest Mar 28, 2018
92a0718
feat(top-app-bar): Update test. Make variable private
williamernest Mar 30, 2018
fb858df
feat(top-app-bar): Merge with master
williamernest Apr 2, 2018
e2288ab
Merge branch 'master' into feat/top-app-bar/add-default-scroll-behavior
williamernest Apr 3, 2018
b6fd60c
feat(top-app-bar): Fix linter
williamernest Apr 3, 2018
c66923c
Merge branch 'master' into feat/top-app-bar/add-default-scroll-behavior
williamernest Apr 4, 2018
d2ef63d
Merge branch 'master' into feat/top-app-bar/add-default-scroll-behavior
williamernest Apr 5, 2018
8ce12e9
feat(top-app-bar): Update for comments
williamernest Apr 5, 2018
1e04327
feat(top-app-bar): Update for comments
williamernest Apr 5, 2018
8f5327f
Merge branch 'master' into feat/top-app-bar/add-default-scroll-behavior
williamernest Apr 6, 2018
ce71bba
Merge branch 'master' into feat/top-app-bar/add-default-scroll-behavior
williamernest Apr 6, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion demos/top-app-bar.html
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
</aside>

<main class="demo-main">
<div>
<div id="content-main" class="mdc-top-app-bar-fixed-adjust">
<p class="demo-paragraph">
Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. Aenean ultricies mi vitae est. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. Aenean ultricies mi vitae est.
</p>
Expand Down Expand Up @@ -200,6 +200,7 @@ <h4 class="mdc-typography--subheading2 mdc-typography--adjust-margin">Short Top
var rightItemEl = document.getElementById('right-item');
var rightSection = document.getElementById('iconSection');
var drawerEl = document.querySelector('.mdc-drawer');
var contentMainEl = document.getElementById('content-main');

var drawer = new mdc.drawer.MDCTemporaryDrawer(drawerEl);
var appBar = mdc.topAppBar.MDCTopAppBar.attachTo(appBarEl);
Expand Down Expand Up @@ -235,6 +236,7 @@ <h4 class="mdc-typography--subheading2 mdc-typography--adjust-margin">Short Top
shortCheckbox.addEventListener('change', function() {
appBarEl.classList[this.checked ? 'add' : 'remove']('mdc-top-app-bar--short');
appBarEl.classList.remove('mdc-top-app-bar--short-has-action-item');
contentMainEl.classList[this.checked ? 'add' : 'remove']('mdc-top-app-bar--short-fixed-adjust');

appBar.destroy();
appBar = mdc.topAppBar.MDCTopAppBar.attachTo(appBarEl);
Expand All @@ -258,8 +260,13 @@ <h4 class="mdc-typography--subheading2 mdc-typography--adjust-margin">Short Top

prominentCheckbox.addEventListener('change', function() {
appBarEl.classList[this.checked ? 'add' : 'remove']('mdc-top-app-bar--prominent');
contentMainEl.classList[this.checked ? 'add' : 'remove']('mdc-top-app-bar--prominent-fixed-adjust');

shortCheckbox.disabled = this.checked;

// Needs to reset the toolbar height for the default scroll behavior
appBar.destroy();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be a good case for a layout method or something to reset the scrolling? I can't think of a use case outside of the demo pages for it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only required for the demo page since we're showcasing several different toolbars on a single page.

appBar = mdc.topAppBar.MDCTopAppBar.attachTo(appBarEl);
});
});
</script>
Expand Down
6 changes: 4 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions packages/mdc-top-app-bar/_mixins.scss
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@
padding-bottom: $mdc-top-app-bar-prominent-mobile-title-bottom-padding;
}
}

.mdc-top-app-bar-fixed-adjust {
margin-top: $mdc-top-app-bar-mobile-row-height;
}
}
}

Expand Down
19 changes: 19 additions & 0 deletions packages/mdc-top-app-bar/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ class MDCTopAppBarAdapter {
*/
hasClass(className) {}

/**
* Adds the specified attribute and value to the root Element.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Update comment to something like:

Sets the specified inline style property on the root Element to the given value.

* @param {string} attribute
* @param {string} value
*/
setStyle(attribute, value) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Rename attribute to property

(See https://css-tricks.com/css-ruleset-terminology/)


/**
* Gets the height of the top app bar.
* @return {number}
*/
getTopAppBarHeight() {}

/**
* Registers an event handler on the navigation icon element for a given event.
* @param {string} type
Expand All @@ -72,6 +85,12 @@ class MDCTopAppBarAdapter {
/** @param {function(!Event)} handler */
deregisterScrollHandler(handler) {}

/** @param {function(!Event)} handler */
registerResizeHandler(handler) {}

/** @param {function(!Event)} handler */
deregisterResizeHandler(handler) {}

/** @return {number} */
getViewportScrollY() {}

Expand Down
8 changes: 7 additions & 1 deletion packages/mdc-top-app-bar/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,10 @@ const cssClasses = {
SHORT_COLLAPSED_CLASS: 'mdc-top-app-bar--short-collapsed',
};

export {strings, cssClasses};
/** @enum {number} */
const numbers = {
MAX_TOP_APP_BAR_HEIGHT: 128,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swap these for alpha order

DEBOUNCE_THROTTLE_RESIZE_TIME: 100,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For time values stored as integers, it's usually a good idea to include the unit of measurement in the variable name.

E.g., DEBOUNCE_THROTTLE_RESIZE_TIME_MS

};

export {strings, cssClasses, numbers};
19 changes: 14 additions & 5 deletions packages/mdc-top-app-bar/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
* limitations under the License.
*/

import {strings, cssClasses} from './constants';
import {strings, cssClasses, numbers} from './constants';
import MDCTopAppBarAdapter from './adapter';
import MDCFoundation from '@material/base/foundation';

/**
* @extends {MDCFoundation<!MDCTopAppBarAdapter>}
*/
class MDCTopAppBarFoundation extends MDCFoundation {
class MDCTopAppBarBaseFoundation extends MDCFoundation {
/** @return enum {string} */
static get strings() {
return strings;
Expand All @@ -33,6 +33,11 @@ class MDCTopAppBarFoundation extends MDCFoundation {
return cssClasses;
}

/** @return enum {number} */
static get numbers() {
return numbers;
}

/**
* {@see MDCTopAppBarAdapter} for typing information on parameters and return
* types.
Expand All @@ -43,11 +48,15 @@ class MDCTopAppBarFoundation extends MDCFoundation {
hasClass: (/* className: string */) => {},
addClass: (/* className: string */) => {},
removeClass: (/* className: string */) => {},
setStyle: (/* attribute: string, value: string */) => {},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Rename attribute to property

(See https://css-tricks.com/css-ruleset-terminology/)

getTopAppBarHeight: () => {},
registerNavigationIconInteractionHandler: (/* type: string, handler: EventListener */) => {},
deregisterNavigationIconInteractionHandler: (/* type: string, handler: EventListener */) => {},
notifyNavigationIconClicked: () => {},
registerScrollHandler: (/* handler: EventListener */) => {},
deregisterScrollHandler: (/* handler: EventListener */) => {},
registerResizeHandler: (/* handler: EventListener */) => {},
deregisterResizeHandler: (/* handler: EventListener */) => {},
getViewportScrollY: () => /* number */ 0,
getTotalActionItems: () => /* number */ 0,
});
Expand All @@ -56,8 +65,8 @@ class MDCTopAppBarFoundation extends MDCFoundation {
/**
* @param {!MDCTopAppBarAdapter} adapter
*/
constructor(adapter) {
super(Object.assign(MDCTopAppBarFoundation.defaultAdapter, adapter));
constructor(/** @type {!MDCTopAppBarAdapter} */ adapter) {
super(Object.assign(MDCTopAppBarBaseFoundation.defaultAdapter, adapter));

this.navClickHandler_ = () => this.adapter_.notifyNavigationIconClicked();
}
Expand All @@ -71,4 +80,4 @@ class MDCTopAppBarFoundation extends MDCFoundation {
}
}

export default MDCTopAppBarFoundation;
export default MDCTopAppBarBaseFoundation;
14 changes: 10 additions & 4 deletions packages/mdc-top-app-bar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
*/

import MDCTopAppBarAdapter from './adapter';
import MDCTopAppBarFoundation from './foundation';
import MDCTopAppBarBaseFoundation from './foundation';
import MDCComponent from '@material/base/component';
import {MDCRipple} from '@material/ripple/index';
import {cssClasses, strings} from './constants';
import * as util from './util';
import MDCShortTopAppBarFoundation from './short/foundation';
import MDCTopAppBarFoundation from './standard/foundation';

/**
* @extends {MDCComponent<!MDCTopAppBarFoundation>}
* @extends {MDCComponent<!MDCTopAppBarBaseFoundation>}
* @final
*/
class MDCTopAppBar extends MDCComponent {
Expand Down Expand Up @@ -68,14 +69,16 @@ class MDCTopAppBar extends MDCComponent {
}

/**
* @return {!MDCTopAppBarFoundation}
* @return {!MDCTopAppBarBaseFoundation}
*/
getDefaultFoundation() {
/** @type {!MDCTopAppBarAdapter} */
const adapter = /** @type {!MDCTopAppBarAdapter} */ (Object.assign({
hasClass: (className) => this.root_.classList.contains(className),
addClass: (className) => this.root_.classList.add(className),
removeClass: (className) => this.root_.classList.remove(className),
setStyle: (attribute, value) => this.root_.style.setProperty(attribute, value),
getTopAppBarHeight: () => this.root_.clientHeight,
registerNavigationIconInteractionHandler: (evtType, handler) => {
if (this.navIcon_) {
this.navIcon_.addEventListener(evtType, handler);
Expand All @@ -91,12 +94,15 @@ class MDCTopAppBar extends MDCComponent {
},
registerScrollHandler: (handler) => window.addEventListener('scroll', handler, util.applyPassive()),
deregisterScrollHandler: (handler) => window.removeEventListener('scroll', handler),
registerResizeHandler: (handler) => window.addEventListener('resize', handler, util.applyPassive()),
deregisterResizeHandler: (handler) => window.removeEventListener('resize', handler),
getViewportScrollY: () => window.pageYOffset,
getTotalActionItems: () =>
this.root_.querySelectorAll(strings.ACTION_ITEM_SELECTOR).length,
})
);

/** @type {!MDCTopAppBarBaseFoundation} */
let foundation;
if (this.root_.classList.contains(cssClasses.SHORT_CLASS)) {
foundation = new MDCShortTopAppBarFoundation(adapter);
Expand All @@ -108,4 +114,4 @@ class MDCTopAppBar extends MDCComponent {
}
}

export {MDCTopAppBar, MDCTopAppBarFoundation, MDCShortTopAppBarFoundation, util};
export {MDCTopAppBar, MDCTopAppBarBaseFoundation, MDCTopAppBarFoundation, MDCShortTopAppBarFoundation, util};
15 changes: 14 additions & 1 deletion packages/mdc-top-app-bar/mdc-top-app-bar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
@include mdc-top-app-bar-icon-ink-color(text-primary-on-primary);

display: flex;
position: relative;
position: fixed;
flex-direction: column;
justify-content: space-between;
box-sizing: border-box;
Expand Down Expand Up @@ -145,5 +145,18 @@
}
}

// stylelint-disable-next-line plugin/selector-bem-pattern
.mdc-top-app-bar-fixed-adjust {
margin-top: $mdc-top-app-bar-row-height;
}

.mdc-top-app-bar--short-fixed-adjust {
margin-top: $mdc-top-app-bar-mobile-row-height;
}

.mdc-top-app-bar--prominent-fixed-adjust {
margin-top: $mdc-top-app-bar-prominent-row-height;
}

// Mobile Styles
@include mdc-top-app-bar-mobile-breakpoint_;
9 changes: 5 additions & 4 deletions packages/mdc-top-app-bar/short/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
* limitations under the License.
*/

import {cssClasses} from '../constants';
import MDCTopAppBarAdapter from '../adapter';
import MDCTopAppBarFoundation from '../foundation';
import MDCTopAppBarBaseFoundation from '../foundation';
import {cssClasses} from '../constants';

/**
* @extends {MDCTopAppBarFoundation<!MDCShortTopAppBarFoundation>}
* @extends {MDCTopAppBarBaseFoundation<!MDCShortTopAppBarFoundation>}
* @final
*/
class MDCShortTopAppBarFoundation extends MDCTopAppBarFoundation {
class MDCShortTopAppBarFoundation extends MDCTopAppBarBaseFoundation {
/**
* @param {!MDCTopAppBarAdapter} adapter
*/
Expand Down Expand Up @@ -58,6 +58,7 @@ class MDCShortTopAppBarFoundation extends MDCTopAppBarFoundation {
/**
* Scroll handler for applying/removing the collapsed modifier class
* on the short top app bar.
* @private
*/
shortAppBarScrollHandler_() {
const currentScroll = this.adapter_.getViewportScrollY();
Expand Down
Loading