fix(aio): fix SideNav height and auto-close when necessary #21538

gkalpak · 2018-01-15T14:15:22Z

This PR addresses points 2 and 3 of #21520.

mary-poppins · 2018-01-15T14:22:33Z

You can preview 1633bad at https://pr21538-1633bad.ngbuilds.io/.

mary-poppins · 2018-01-18T13:12:11Z

You can preview 9d875f5 at https://pr21538-9d875f5.ngbuilds.io/.

petebacondarwin · 2018-01-19T16:21:10Z

aio/src/app/app.component.spec.ts

+            navigateTo(doc);
+            expect(sidenav.opened).toBe(false);
+
+            resizeTo(sideBySideBreakPoint + 1);


In the beforeEach we resize to sideBySideBreakpoint + 1 so isn't this line a noop?

Ooops! I meant resizeTo(sideBySideBreakPoint - 1) 😇

petebacondarwin · 2018-01-19T16:23:01Z

aio/src/app/app.component.spec.ts

+        const sidenavDocs = ['api/a/b/c/d', 'guide/pipes'];
+        const nonSidenavDocs = ['features', 'about'];
+
+        beforeEach(() => resizeTo(sideBySideBreakPoint + 1));  // initially side-by-side


it would be easier to follow if this was moved into the actual it clauses

petebacondarwin · 2018-01-19T16:29:19Z

aio/src/app/app.component.spec.ts

-        navigateTo('api/a/b/c/d');
-        expect(sidenav.opened).toBe(false);
-      });
+          it('should close when click in gray content area overlay', () => {


click -> clicking

petebacondarwin · 2018-01-19T16:29:50Z

aio/src/app/app.component.spec.ts

-        navigateTo('features');
-        expect(sidenav.opened).toBe(false);
-      });
+          it('should close when nav to another guide page', () => {


nav -> navigating

petebacondarwin · 2018-01-19T17:01:02Z

aio/src/app/app.component.spec.ts

        locationService.go(path);
        component.updateSideNav();
+        if (toggleSidenav) {
+          hamburger.click();
+        }


I think it would make the tests more readable if the toggle was in its own helper function.
Note that this could then be used in the other tests that are currently doing this the "long" way below.

Actually, the navigateTo() helper can already be used in these other tests as well, but I totally agree that making it more explicit (instead of a boolean arg in an unrelated helper 😁) is worth the extra fixture.detectChanges() 💯
Moved to a separate helper.

petebacondarwin

Only this really: https://github.com/angular/angular/pull/21538/files#r162666308

Since we specify `bottom: 0`, specifying the height is unnecessary and leads to wrong height (unless updated) on narrow screens where the topbar height is decreased. Partly addresses angular#21520.

Partly addresses angular#21520.

gkalpak · 2018-01-22T11:50:29Z

Updated. PTAL

mary-poppins · 2018-01-22T11:56:05Z

You can preview 67054e1 at https://pr21538-67054e1.ngbuilds.io/.

googlebot · 2018-01-25T20:59:29Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

mary-poppins · 2018-01-25T21:06:05Z

You can preview 2d63760 at https://pr21538-2d63760.ngbuilds.io/.

mary-poppins · 2018-01-25T22:14:23Z

You can preview b5f6ef5 at https://pr21538-b5f6ef5.ngbuilds.io/.

petebacondarwin · 2018-01-26T07:56:05Z

Not sure what happened with the CLA, @gkalpak ??

gkalpak · 2018-01-26T09:12:01Z

Sorry, about the CLA. We were trying something out with @jbogarthyde and @dennispbrown 😁

Since we specify `bottom: 0`, specifying the height is unnecessary and leads to wrong height (unless updated) on narrow screens where the topbar height is decreased. Partly addresses #21520. PR Close #21538

Partly addresses #21520. PR Close #21538

Since we specify `bottom: 0`, specifying the height is unnecessary and leads to wrong height (unless updated) on narrow screens where the topbar height is decreased. Partly addresses angular#21520. PR Close angular#21538

) Partly addresses angular#21520. PR Close angular#21538

Since we specify `bottom: 0`, specifying the height is unnecessary and leads to wrong height (unless updated) on narrow screens where the topbar height is decreased. Partly addresses angular#21520. PR Close angular#21538

) Partly addresses angular#21520. PR Close angular#21538

angular-automatic-lock-bot · 2019-09-13T06:20:47Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Jan 15, 2018

gkalpak added type: bug/fix action: review The PR is still awaiting reviews from at least one requested reviewer comp: aio target: patch This PR is targeted for the next patch release labels Jan 15, 2018

gkalpak added this to REVIEW in docs-infra Jan 15, 2018

gkalpak mentioned this pull request Jan 15, 2018

The layout on the official site is broken #21520

Closed

gkalpak requested a review from petebacondarwin January 18, 2018 12:59

gkalpak force-pushed the fix-aio-layout branch from 1633bad to 9d875f5 Compare January 18, 2018 13:05

petebacondarwin reviewed Jan 19, 2018

View reviewed changes

petebacondarwin requested changes Jan 19, 2018

View reviewed changes

petebacondarwin added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 19, 2018

gkalpak added 2 commits January 22, 2018 13:06

fix(aio): fix SideNav height on narrow screens

a5e713c

Since we specify `bottom: 0`, specifying the height is unnecessary and leads to wrong height (unless updated) on narrow screens where the topbar height is decreased. Partly addresses angular#21520.

fix(aio): close SideNav on non-sidenav doc on wide screen

67054e1

Partly addresses angular#21520.

gkalpak force-pushed the fix-aio-layout branch from 9d875f5 to 67054e1 Compare January 22, 2018 11:49

gkalpak added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jan 22, 2018

googlebot added cla: no and removed cla: yes labels Jan 25, 2018

gkalpak force-pushed the fix-aio-layout branch from 2d63760 to 67054e1 Compare January 25, 2018 21:09

gkalpak force-pushed the fix-aio-layout branch from b5f6ef5 to 67054e1 Compare January 25, 2018 22:43

petebacondarwin approved these changes Jan 26, 2018

View reviewed changes

petebacondarwin added action: merge The PR is ready for merge by the caretaker cla: yes and removed action: review The PR is still awaiting reviews from at least one requested reviewer cla: no labels Jan 26, 2018

mhevery pushed a commit that referenced this pull request Jan 26, 2018

fix(aio): close SideNav on non-sidenav doc on wide screen (#21538)

67806a7

Partly addresses #21520. PR Close #21538

mhevery closed this in 1b2271a Jan 26, 2018

mhevery pushed a commit that referenced this pull request Jan 26, 2018

fix(aio): close SideNav on non-sidenav doc on wide screen (#21538)

108fa15

Partly addresses #21520. PR Close #21538

gkalpak deleted the fix-aio-layout branch January 29, 2018 13:04

gkalpak removed this from REVIEW in docs-infra Jan 30, 2018

jbogarthyde pushed a commit to jbogarthyde/angular that referenced this pull request Feb 23, 2018

fix(aio): close SideNav on non-sidenav doc on wide screen (angular#21538

c4187c0

) Partly addresses angular#21520. PR Close angular#21538

leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018

fix(aio): close SideNav on non-sidenav doc on wide screen (angular#21538

bd5436c

) Partly addresses angular#21520. PR Close angular#21538

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(aio): fix SideNav height and auto-close when necessary #21538

fix(aio): fix SideNav height and auto-close when necessary #21538

gkalpak commented Jan 15, 2018

mary-poppins commented Jan 15, 2018

mary-poppins commented Jan 18, 2018

petebacondarwin Jan 19, 2018

gkalpak Jan 22, 2018

petebacondarwin Jan 19, 2018

petebacondarwin Jan 19, 2018

petebacondarwin Jan 19, 2018

petebacondarwin Jan 19, 2018

gkalpak Jan 22, 2018

petebacondarwin left a comment

gkalpak commented Jan 22, 2018

mary-poppins commented Jan 22, 2018

googlebot commented Jan 25, 2018

mary-poppins commented Jan 25, 2018

mary-poppins commented Jan 25, 2018

petebacondarwin commented Jan 26, 2018

gkalpak commented Jan 26, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

fix(aio): fix SideNav height and auto-close when necessary #21538

fix(aio): fix SideNav height and auto-close when necessary #21538

Conversation

gkalpak commented Jan 15, 2018

mary-poppins commented Jan 15, 2018

mary-poppins commented Jan 18, 2018

petebacondarwin Jan 19, 2018

Choose a reason for hiding this comment

gkalpak Jan 22, 2018

Choose a reason for hiding this comment

petebacondarwin Jan 19, 2018

Choose a reason for hiding this comment

petebacondarwin Jan 19, 2018

Choose a reason for hiding this comment

petebacondarwin Jan 19, 2018

Choose a reason for hiding this comment

petebacondarwin Jan 19, 2018

Choose a reason for hiding this comment

gkalpak Jan 22, 2018

Choose a reason for hiding this comment

petebacondarwin left a comment

Choose a reason for hiding this comment

gkalpak commented Jan 22, 2018

mary-poppins commented Jan 22, 2018

googlebot commented Jan 25, 2018

mary-poppins commented Jan 25, 2018

mary-poppins commented Jan 25, 2018

petebacondarwin commented Jan 26, 2018

gkalpak commented Jan 26, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019