Skip to content

🚮 Clean up dead amp-sidebar code within stories #36178

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

Merged
gmajoulet merged 14 commits into from
Sep 30, 2021
Merged

🚮 Clean up dead amp-sidebar code within stories #36178

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
366e4a1
Remove amp-sidebar code from extensions/amp-story
coreymasanto Sep 27, 2021
627fc3e
Remove amp-sidebar visual tests from examples/visual-tests/amp-story
coreymasanto Sep 27, 2021
313e517
Remove reference to amp-sidebar from amp-story-interactive README
coreymasanto Sep 27, 2021
ca235c1
Remove remaining amp-sidebar logic from extensions/amp-story
coreymasanto Sep 27, 2021
0696fc5
Remove amp-sidebar logic from amp-story-system-layer code
coreymasanto Sep 27, 2021
973910d
Remove amp-sidebar code from amp-story-store-service.js
coreymasanto Sep 27, 2021
3b00f1a
Remove amp-sidebar storybook JS files
coreymasanto Sep 27, 2021
ef2a7a8
Merge branch 'main' into amp-sidebar-cleanup
coreymasanto Sep 27, 2021
fc909a0
A few missed deletions in test-amp-story and build-system/ caught by …
coreymasanto Sep 27, 2021
cad9568
Revert extensions/amp-sidebar/*/storybook removal from forbidden terms
coreymasanto Sep 29, 2021
197c365
Add back the amp-sidebar storybook files
coreymasanto Sep 30, 2021
6c17c6d
Merge branch 'main' into amp-sidebar-cleanup
coreymasanto Sep 30, 2021
307c201
Add newline at end of each storybook file
coreymasanto Sep 30, 2021
b7d4a6f
Run amp get-zindex --fix
coreymasanto Sep 30, 2021
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
Next Next commit
Remove amp-sidebar code from extensions/amp-story
  • Loading branch information
@coreymasanto
coreymasanto committed Sep 27, 2021
commit 366e4a14bff3916736df9656942a1c56a1d9fa22
4 changes: 0 additions & 4 deletions extensions/amp-story/1.0/amp-story-desktop-user-overridable.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,3 @@
.i-amphtml-story-background {
background-color: transparent;
}

amp-sidebar {
background-color: #ffffff;
}
27 changes: 0 additions & 27 deletions extensions/amp-story/1.0/amp-story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,6 @@ export class AmpStory extends AMP.BaseElement {
const initialPageId = this.getInitialPageId_();

this.buildSystemLayer_(initialPageId);
this.initializeSidebar_();
this.setThemeColor_();

const storyLayoutPromise = Promise.all([
Expand Down Expand Up @@ -2454,32 +2453,6 @@ export class AmpStory extends AMP.BaseElement {
this.switchTo_(targetPage.element.id, direction);
}

/**
* Checks for the presence of a sidebar. If a sidebar does exist, then an icon
* permitting for the opening/closing of the sidebar is shown.
* @private
*/
initializeSidebar_() {
this.sidebar_ = this.element.querySelector('amp-sidebar');
if (!this.sidebar_) {
return;
}

this.mutateElement(() => {
this.sidebar_.classList.add(SIDEBAR_CLASS_NAME);
});

this.initializeOpacityMask_();
this.storeService_.dispatch(Action.TOGGLE_HAS_SIDEBAR, !!this.sidebar_);

const actions = [
{tagOrTarget: 'AMP-SIDEBAR', method: 'open'},
{tagOrTarget: 'AMP-SIDEBAR', method: 'close'},
{tagOrTarget: 'AMP-SIDEBAR', method: 'toggle'},
];
this.storeService_.dispatch(Action.ADD_TO_ACTIONS_ALLOWLIST, actions);
}

/**
* Checks for the the storyNavigationPath stack in the history.
* @private
Expand Down
64 changes: 0 additions & 64 deletions extensions/amp-story/1.0/test/test-amp-story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,70 +790,6 @@ describes.realWin(
});
});

describe('amp-story custom sidebar', () => {
it('should show the sidebar control if a sidebar exists', async () => {
await createStoryWithPages(2, ['cover', 'page-1']);

const sidebar = win.document.createElement('amp-sidebar');
story.element.appendChild(sidebar);

await story.layoutCallback();
expect(story.storeService_.get(StateProperty.HAS_SIDEBAR_STATE)).to.be
.true;
});
});

it('should open the sidebar on button click', async () => {
await createStoryWithPages(2, ['cover', 'page-1']);

const sidebar = win.document.createElement('amp-sidebar');
story.element.appendChild(sidebar);

const executeSpy = env.sandbox.spy();
env.sandbox.stub(Services, 'actionServiceForDoc').returns({
setAllowlist: () => {},
trigger: () => {},
execute: executeSpy,
});

story.buildCallback();
await story.layoutCallback();
story.storeService_.dispatch(Action.TOGGLE_SIDEBAR, true);
expect(executeSpy).to.have.been.calledWith(
story.sidebar_,
'open',
null,
null,
null,
null,
ActionTrust.HIGH
);
});

it('should unpause the story when the sidebar is closed', async () => {
await createStoryWithPages(2, ['cover', 'page-1']);

const sidebar = win.document.createElement('amp-sidebar');
story.element.appendChild(sidebar);

env.sandbox.stub(Services, 'actionServiceForDoc').returns({
setAllowlist: () => {},
trigger: () => {},
execute: () => {
sidebar.setAttribute('open', '');
},
});

story.buildCallback();
await story.layoutCallback();
story.storeService_.dispatch(Action.TOGGLE_SIDEBAR, true);
await Promise.resolve();
story.sidebar_.removeAttribute('open');
await Promise.resolve();
expect(story.storeService_.get(StateProperty.SIDEBAR_STATE)).to.be
.false;
});

it('should add previous visited attribute', async () => {
env.sandbox
.stub(utils, 'setAttributeInMutate')
Expand Down