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.

Sign up for GitHub

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

Jump to bottom

[amp-sidebar 1.0] [Toolbar] Implemented toolbar's "target" attribute #10126

Merged
merged 12 commits into from Jun 29, 2017
Merged

[amp-sidebar 1.0] [Toolbar] Implemented toolbar's "target" attribute #10126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8c297cf
Implemented toolbar's "target" attribute, with a quick demo
torch2424 Jun 23, 2017
e84656f
Finished up syncing to latest master
torch2424 Jun 27, 2017
2803511
Fixed check-types error
torch2424 Jun 27, 2017
100c566
Wrote tests for toolbar target
torch2424 Jun 27, 2017
bfc2932
Fixed linting merge errors
torch2424 Jun 27, 2017
8e9a018
Fixed toolbar class being applied to targets
torch2424 Jun 27, 2017
9ada4bb
Cleaned up toolbar buuild logic
torch2424 Jun 27, 2017
37746ce
Added a comment on some clarification
torch2424 Jun 27, 2017
8fe474a
Fixed the missing ownerDocument on the container
torch2424 Jun 27, 2017
57bc489
Allowed for having targets within other toolbars
torch2424 Jun 29, 2017
dbc640d
Cleaned up the test manual to look beautiful
torch2424 Jun 29, 2017
7b6da77
Removed unneccasry spacing
torch2424 Jun 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions extensions/amp-sidebar/1.0/amp-sidebar.js
View file
Open in desktop
Expand Up @@ -112,10 +112,7 @@ export class AmpSidebar extends AMP.BaseElement {
.call(this.element.querySelectorAll('nav[toolbar]'), 0);
toolbarElements.forEach(toolbarElement => {
try {
const toolbar = new Toolbar(toolbarElement, this.win, this.vsync_);
this.element.parentElement
.insertBefore(toolbar.build(), this.element);
this.toolbars_.push(toolbar);
this.toolbars_.push(new Toolbar(toolbarElement, this.win, this.vsync_));
} catch (e) {
user().error(TAG, 'Failed to instantiate toolbar', e);
}
Expand Down
104 changes: 93 additions & 11 deletions extensions/amp-sidebar/1.0/test/test-toolbar.js
View file
Open in desktop
Expand Up @@ -25,7 +25,7 @@
const DEFAULT_TOOLBAR_MEDIA = '(min-width: 768px)';

/** @const */
const TOOLBAR_CLASS = 'i-amphtml-toolbar';
const TOOLBAR_ELEMENT_CLASS = 'i-amphtml-toolbar';


adopt(window);
Expand Down Expand Up @@ -69,6 +69,12 @@
if (toolbarObj.toolbarOnlyOnNav) {
navToolbar.setAttribute('toolbar-only', '');
}
if (toolbarObj.target) {
const toolbarTarget = iframe.doc.createElement('div');
toolbarTarget.setAttribute('id', toolbarObj.target);
iframe.win.document.body.appendChild(toolbarTarget);
navToolbar.setAttribute('target', toolbarObj.target);
}
const toolbarList = iframe.doc.createElement('ul');
for (let i = 0; i < 3; i++) {
const li = iframe.doc.createElement('li');
Expand All @@ -77,9 +83,7 @@
}
navToolbar.appendChild(toolbarList);
toolbarContainerElement.appendChild(navToolbar);
const toolbar = new Toolbar(navToolbar, iframe.win, vsync);
toolbarContainerElement.appendChild(toolbar.build());
toolbars.push(toolbar);
toolbars.push(new Toolbar(navToolbar, iframe.win, vsync));
});

return {iframe, toolbarContainerElement, toolbars};
Expand Down Expand Up @@ -107,7 +111,7 @@
const toolbars = obj.toolbars;
const toolbarElements = Array.prototype
.slice.call(obj.toolbarContainerElement.ownerDocument
.getElementsByClassName(TOOLBAR_CLASS), 0);
.getElementsByClassName(TOOLBAR_ELEMENT_CLASS), 0);
resizeIframeToWidth(obj.iframe, '1px', () => {
expect(toolbarElements.length).to.be.above(0);
toolbars.forEach(toolbar => {
Expand All @@ -124,8 +128,8 @@
return getToolbars([{}]).then(obj => {
const toolbars = obj.toolbars;
const toolbarElements = Array.prototype
.slice.call(obj.toolbarContainerElement
.getElementsByClassName(TOOLBAR_CLASS), 0);
.slice.call(obj.toolbarContainerElement.ownerDocument
.getElementsByClassName(TOOLBAR_ELEMENT_CLASS), 0);
resizeIframeToWidth(obj.iframe, '4000px', () => {
expect(toolbarElements.length).to.be.above(0);
toolbars.forEach(toolbar => {
Expand All @@ -142,8 +146,8 @@
return getToolbars([{}]).then(obj => {
const toolbars = obj.toolbars;
const toolbarElements = Array.prototype
.slice.call(obj.toolbarContainerElement
.getElementsByClassName(TOOLBAR_CLASS), 0);
.slice.call(obj.toolbarContainerElement.ownerDocument
.getElementsByClassName(TOOLBAR_ELEMENT_CLASS), 0);
resizeIframeToWidth(obj.iframe, '4000px', () => {
expect(toolbarElements.length).to.be.above(0);
toolbars.forEach(toolbar => {
Expand All @@ -160,8 +164,8 @@
return getToolbars([{}]).then(obj => {
const toolbars = obj.toolbars;
const toolbarElements = Array.prototype
.slice.call(obj.toolbarContainerElement
.getElementsByClassName(TOOLBAR_CLASS), 0);
.slice.call(obj.toolbarContainerElement.ownerDocument
.getElementsByClassName(TOOLBAR_ELEMENT_CLASS), 0);
resizeIframeToWidth(obj.iframe, '4000px', () => {
toolbars.forEach(toolbar => {
toolbar.onLayoutChange();
Expand All @@ -178,6 +182,83 @@
});
});

it('toolbar header should be hidden for a \
non-matching window size for DEFAULT_TOOLBAR_MEDIA', () => {
return getToolbars([{}]).then(obj => {
const toolbars = obj.toolbars;
const toolbarElements = Array.prototype
.slice.call(obj.toolbarContainerElement.ownerDocument
.getElementsByClassName(TOOLBAR_ELEMENT_CLASS), 0);
resizeIframeToWidth(obj.iframe, '1px', () => {
expect(toolbarElements.length).to.be.above(0);
toolbars.forEach(toolbar => {
toolbar.onLayoutChange();
});
expect(toolbarElements[0].parentElement.style.display)
.to.be.equal('none');
});
});
});

it('toolbar should be placed into a target, with the \
target attrbiute', () => {
const targetId = 'toolbar-target';
return getToolbars([{
target: targetId,
}]).then(obj => {
const toolbars = obj.toolbars;
const toolbarTargetElements = Array.prototype
.slice.call(obj.iframe.win.document.body
.querySelectorAll(`#${targetId} > nav[toolbar]`), 0);
expect(toolbars.length).to.be.equal(1);
expect(toolbarTargetElements.length).to.be.equal(1);
});
});

it('toolbar should be placed into a target, and shown for a \
matching window size for DEFAULT_TOOLBAR_MEDIA', () => {
const targetId = 'toolbar-target';
return getToolbars([{
target: targetId,
}]).then(obj => {
const toolbars = obj.toolbars;
const toolbarTargets = Array.prototype
.slice.call(obj.iframe.win.document.body
.querySelectorAll(`#${targetId}`), 0);
resizeIframeToWidth(obj.iframe, '4000px', () => {
toolbars.forEach(toolbar => {
toolbar.onLayoutChange();
});
expect(toolbars.length).to.be.equal(1);
expect(toolbarTargets.length).to.be.equal(1);
expect(toolbarTargets[0].style.display)
.to.be.equal('');
});
});
});

it('toolbar should be placed into a target, and hidden for a \
non-matching window size for DEFAULT_TOOLBAR_MEDIA', () => {
const targetId = 'toolbar-target';
return getToolbars([{
target: targetId,
}]).then(obj => {
const toolbars = obj.toolbars;
const toolbarTargets = Array.prototype
.slice.call(obj.iframe.win.document.body
.querySelectorAll(`#${targetId}`), 0);
resizeIframeToWidth(obj.iframe, '200px', () => {
toolbars.forEach(toolbar => {
toolbar.onLayoutChange();
});
expect(toolbars.length).to.be.equal(1);
expect(toolbarTargets.length).to.be.equal(1);
expect(toolbarTargets[0].style.display)
.to.be.equal('none');
});
});
});

it('should hide <nav toolbar> elements with toolbar-only, \
inside the sidebar, but not inside the toolbar, for a matching \
window size for DEFAULT_TOOLBAR_MEDIA', () => {
Expand All @@ -202,6 +283,7 @@
});
});


it('toolbar should be in the hidden state \
when it is not being displayed', () => {
return getToolbars([{}]).then(obj => {
Expand Down
49 changes: 37 additions & 12 deletions extensions/amp-sidebar/1.0/toolbar.js
View file
Open in desktop
Expand Up @@ -75,6 +75,7 @@ export class Toolbar {
Array.prototype.slice.call(toolbarOnlyQuery, 0);
}
}
this.buildCallback_();
}

/**
Expand All @@ -98,22 +99,46 @@ export class Toolbar {
}

/**
* Private function to build the DOM element for the toolbar, and return the built fragment
* @public
* Private function to build the DOM element for the toolbar
* @private
*/
build() {
const fragment = this.win_
.document.createDocumentFragment();
this.targetElement_ =
this.win_.document.createElement('header');
this.targetElement_.classList.add(TOOLBAR_TARGET_CLASS);
//Place the elements into the target
buildCallback_() {
this.toolbarClone_ = this.toolbarDOMElement_.cloneNode(true);
this.toolbarClone_.classList.add(TOOLBAR_ELEMENT_CLASS);
// Addend "_toolbar" to ids on the toolbar clone
const idElementsInClone = Array.prototype.slice
.call(this.toolbarClone_.querySelectorAll('[id]'), 0);
idElementsInClone.forEach(element => {
element.id = `${element.id}_toolbar`;
});
const targetId = this.toolbarDOMElement_.getAttribute('target');
// Set the target element to the toolbar clone if it exists.
const targetElement =
this.win_.document.getElementById(`${targetId}_toolbar`) ||
this.win_.document.getElementById(targetId);
this.targetElement_ = targetElement || this.createTargetElement_();
this.targetElement_.appendChild(this.toolbarClone_);
toggle(this.targetElement_, false);
fragment.appendChild(this.targetElement_);
return fragment;

// Check if the target element was created by us, or already inserted by the user
if (!this.targetElement_.parentElement) {
this.toolbarClone_.classList.add(TOOLBAR_ELEMENT_CLASS);
const fragment = this.win_
.document.createDocumentFragment();
fragment.appendChild(this.targetElement_);
this.body_.appendChild(fragment);
}
}

/**
* Returns a created element that can be used as the target if one does not exist
* @returns {Element}
* @private
*/
createTargetElement_() {
const targetElement =
this.win_.document.createElement('header');
targetElement.classList.add(TOOLBAR_TARGET_CLASS);
return targetElement;
}

/**
Expand Down
36 changes: 13 additions & 23 deletions test/manual/amp-sidebar-1.0.amp.html
View file
Open in desktop
Expand Up @@ -48,8 +48,8 @@
justify-content: center !important;
}

.transparent-bg {
background-color: transparent;
.mrauto {
margin-right: auto;
}

.ampstart-headerbar-nav, .ampstart-headerbar-nav > ul.flex {
Expand All @@ -74,6 +74,12 @@
background-color: #fff;
}

@media (min-width: 931px) {
.ampstart-headerbar-nav {
margin-left: -195px;
}
}

@media (max-width: 930px) {
.ampstart-headerbar-nav > ul.flex {
flex-direction: column;
Expand Down Expand Up @@ -119,18 +125,20 @@
<!-- Toolbar One, basic usage, toolbar-only -->
<!-- Could center with CSS justify-content: space-between, but trying to be minimal -->
<nav toolbar="(min-width: 0px)" class="ampstart-hamburger-nav ampstart-navbar-bg" toolbar-only>
<ul>
<ul class="justify-center">
<li role="button" aria-label="open sidebar" on="tap:header-sidebar.toggle" tabindex="0" class="ampstart-navbar-trigger pl2">
☰
</li>
<li>
<li class="mrauto">
<amp-img src="https://ampstart.com/img/blog/logo.png" width="100" height="61.3" layout="fixed" class="my0 ml2" alt="The Blog"></amp-img>
</li>
<li id="toolbar-nav-target" class="mrauto">
</li>
</ul>
</nav>

<!-- Toolbar Two, min only -->
<nav toolbar="(min-width: 931px)" toolbar-only class="ampstart-headerbar-nav ampstart-nav ampstart-navbar-bg">
<nav toolbar="(min-width: 931px)" target="toolbar-nav-target" toolbar-only class="ampstart-headerbar-nav ampstart-nav ampstart-navbar-bg">
<ul class="list-reset center m0 p0 flex justify-center nowrap">
<li class="ampstart-nav-item ampstart-nav-dropdown">
<!-- Start Dropdown -->
Expand Down Expand Up @@ -266,27 +274,9 @@ <h3 class="ampstart-subtitle">Eric</h3>
<label for="emailid" class="absolute top-0 right-0 bottom-0 left-0">Email</label>
</div>
<!-- End Input-->










<!-- Start Submit -->
<input type="submit" name="submit" value="SUBMIT" id="submit" class="ampstart-btn mb3 ampstart-btn-secondary">
<!-- End Submit -->








</fieldset>
</form>
</div>
Expand Down