docs(app): refactor the Docs App based on the MD prototype #10341

gkalpak · 2014-12-05T14:47:12Z

I incorporated the prototype detailed in gkalpak#3.

Things that are different from the prototype (besides using actual data):

The left sidenav (toc) is not styled properly (or even readably).
Icons are missing (since we are not using FontAwesome). E.g. search-fab, toc-toggle, results
In the right sidenav (search) there are some unwanted horizontal scrollbars.
(This section probably needs rethinking anyway.)
I have implemented a responsiveMenu directive to show the options in the header-menus
(Learn/Develop/Discuss). The purpose is to show the options in a "dropdown-ish" menu on larger
screens and using a BottomSheet on smaller. At the moment a BottomSheet is used on every
screen-size.

Notes:

Still no version-picker and breadcrumbs (I have left the original implementation of those two
commented out).
I have replaced the previous docs.css with a new one containing the style of the prototype.
I have renamed the old one to docs_old.css and left it there temporarily for quick reference.
We need to clean up this and the rest of the old CSS files.
The main content area is totally unstyled and needs some love (a lot actually).

I have also "sprinkled" the code with a few more TODOs/FIXMEs.

(For more details on known issues and missing features take a look at
gkalpak#3)

googlebot · 2014-12-05T14:47:16Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2014-12-05T15:05:18Z

CLAs look good, thanks!

googlebot · 2014-12-05T15:10:40Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2014-12-05T15:19:18Z

CLAs look good, thanks!

gkalpak · 2014-12-05T15:23:17Z

Finally, there :)

/ping @jesselpalmer, @joshkurz, @caitp, @petebacondarwin

petebacondarwin · 2014-12-05T16:33:02Z

Awesome. I'll take a look shortly
On 5 Dec 2014 15:23, "Georgios Kalpakas" notifications@github.com wrote:

Finally, there :)

/ping @jesselpalmer https://github.com/jesselpalmer, @joshkurz
https://github.com/joshkurz, @caitp https://github.com/caitp,
@petebacondarwin https://github.com/petebacondarwin

—
Reply to this email directly or view it on GitHub
#10341 (comment).

petebacondarwin · 2014-12-05T16:39:13Z

docs/app/src/directives.js

+          targetElem.scrollIntoView();
+        }
+      }
+    }


Why are you not using $anchorScroll?

Because scrolling does not happen on the window level (but a nested element), we don't want to scroll to top. Thus we need to tell $anchorScroll which element we want it to scroll to and the only way is to set $location.hash to its ID (e.g. top-anchor). And that wouldn't look nice imo.

BTW, I think there is an awesome PR that adds support for providing an explicit hash to $anchorScroll, in case we don't want it to scroll to $location.hash().
There it is: #9596 😉

jesselpalmer · 2014-12-05T19:03:14Z

docs/app/assets/css/docs.css

 }

-.type-hint-date {
-  background:pink;
+.md-tile-left .fa {


Consider renaming the 'fa' class. I can't really tell what this is just by reading it.

This is the FontAwesome base class (so can't rename it).
I am leaving them for now (in case we choose to temporarily use FontAwesome, until materlia design's icon font is released.

We can remove them eventually.

Cool, or should I say awesome :)

jesselpalmer · 2014-12-05T19:07:29Z

docs/app/assets/css/docs.css

-
-.view-source {
-  margin-right:10px;
+#sidenav-toc.md-locked-open[flex="33"] {


Add return.

I incorporated the prototype detailed in #3. Things that are different from the prototype (besides using actual data): * The left sidenav (toc) is not styled properly (or even readably). * The subheaders in the left sidenav are not "stickying" properly (only `ng` becomes sticky). * Icons are missing (since we are not using FontAwesome). E.g. search-fab, toc-toggle, results * In the right sidenav (search) there are some unwanted horizontal scrollbars. (This section probably needs rethinking anyway.) * I have implemented a `responsiveMenu` directive to show the options in the header-menus (Learn/Develop/Discuss). The purpose is to show the options in a "dropdown-ish" menu on larger screens and using a BottomSheet on smaller. At the moment a BottomSheet is used on every screen-size. Notes: - Still no version-picker and breadcrumbs (I have left the original implementation of those two commented out). - I have replaced the previous docs.css with a new one containing the style of the prototype. I have renamed the old one to `docs_old.css` and left it there temporarily for quick reference. We need to clean up this and the rest of the old CSS files. - The main content area is totally unstyled and needs some love (a lot actually). I have also "sprinkled" the code with a few more TODOs/FIXMEs. (For more details on known issues and missing features take a look at #3)

gkalpak · 2014-12-05T23:17:51Z

Updated the PR based on the comments.
/cc @jesselpalmer, @joshkurz, @caitp, @petebacondarwin

caitp · 2014-12-06T03:49:46Z

So, just checked it out --- I think it's a reasonable baseline to start with --- there is that huge list of TODOs that we'll want to work on, but I think we could land this to start with, so that Jesse could start working on individual design aspects, and we could start trying to improve the various features in isolation

jesselpalmer · 2014-12-06T04:29:34Z

@caitp Sounds good to me.

gkalpak · 2014-12-06T16:02:53Z

BTW, some tests have been broken:

E2E tests (both jqlite and jQuery), examples 89 and 90 fail because they rely on Bootstrap's glyphicons (which have been removed). We could modify them to make them pass, but I suggest we leave them until we sort out the icons issue, so we can replace the glyphicons icons with material icons.
I think we can just ignore them failing (temporarily).
Units tests for the Docs app (task tests:docs) fail, because:
The DocsContoller module, depends on the new ViewUtils module, which in turn depends on the ngMaterial module (which has a few dependencies of its own: ngAria, ngAnimate, Hammer.js).
Since Karma only loads angular.js and angular-mocks.js (plus the source and test files for the Docs app), there is no ngMaterial, causing ViewUtils to fail loading, which causes DocController to fail loading.

On the long term, this issue might be resolved by structuring the app better, so that each controller has a single area of responsibility (e.g. right now the DocsControllers does anything from setting up Google Analytics, to watching the path, to determining nav-item classes, to setting the current version, to exposing the openPlunkr service).
The question is, how should we handle the failing tests in the meantime ?

Options off the top of my head:
1. Have karma load all necessary files.
2. Ignore the failing tests.
3. Disable the failing tests (until we have more complete test coverage).
4. Remove the ViewUtils dependency from the DocsController module (since it is not used from the failing test-suite). _{(I don't really like this option.)}

jesselpalmer · 2014-12-06T18:44:27Z

I like the disable failing test option. That way we can just update the failing tests after the page is more stable and remove whichever ones we don't need anymore.

caitp · 2014-12-06T20:54:17Z

We can just disable the docsapp tests for now on the materialize branch --- but yes we will need to re-enable them before shipping this

caitp · 2014-12-06T20:55:01Z

but actually, since we will need to update them anyways, it's probably better to fix them as you go

gkalpak · 2014-12-07T10:58:08Z

I pushed a few changes (in a new commit for easier review; should probably be squashed when merging):

Fixed some styling-related issues on the left sidenav
Added FontAwesome (as a temporary solution).
(Wrapped some lines at 100chars in the tests.)

/fyi @jesselpalmer, @joshkurz, @caitp, @petebacondarwin

Since the structure was a little different than the prototype, classes/styles where not applied correctly to the links in the left sidenav. This commit fixes the issue. I also added FontAwesome, in order to be able to display icons until the Material Design icon-font is released. Since this is a temporary meassure, I didn't added as a bower dependency; just added a `font-awesome` folder in assets and added `font-awesome/css/font-awesome.css` to the stylesheets of each deployment (under `docs/config/services/deployments/`.

gkalpak · 2014-12-08T10:21:57Z

@caitp: Making sure I understood you correctly: We should ignore e2e tests failing for examples 89 and 90 (temporarily) and disable unit-testing for the Docs app (temporarily again) ?

Should I make the changes in this PR or a separate one ?

_{(@caitp: Will you merge this (as soon as we sort out the above) or shall I do it ?)}

caitp · 2014-12-08T11:52:29Z

I think you should fix the e2e tests so that it's easier to merge into master later. I was going to do it myself, but you are welcome to do it

Due to the recent changes ragarding the Docs app "Materialization", some tests where failing for the following reasons: 1. JQuery wasn't being copied to 'build/docs/components/'. 2. E2E examples 89 and 90 were relying on the Bootstrap's glyphicons. Temporarily replaced with FontAwesome icons; to be replaced with Material Design iconfont, when ready. 3. `ngMaterial` (and its various dependencies) weren't loaded by Karma during unit testing (causing modules depending on it to fail). Added a fake `ngMaterial` module (as a temporary solution).

gkalpak · 2014-12-08T14:06:43Z

Added a new commit (gkalpak@caed914) fixing test-related errors (more details in the commit's message).
All tests should pass now (waiting to see what Travis has to say on that).

caitp · 2014-12-09T01:48:18Z

Closed by 8edece2...3d5af88

googlebot added the cla: no label Dec 5, 2014

gkalpak force-pushed the materialize-2 branch 2 times, most recently from 2852225 to 1901974 Compare December 5, 2014 15:05

googlebot added cla: yes and removed cla: no labels Dec 5, 2014

gkalpak force-pushed the materialize-2 branch from 1901974 to c78f1ee Compare December 5, 2014 15:10

googlebot added cla: no and removed cla: yes labels Dec 5, 2014

docs(app): minor refactoring of 'docs/app/src/*'

e674142

gkalpak force-pushed the materialize-2 branch from c78f1ee to fb3e6bd Compare December 5, 2014 15:19

googlebot added cla: yes and removed cla: no labels Dec 5, 2014

petebacondarwin reviewed Dec 5, 2014
View reviewed changes

caitp added this to the Materialize The Docs milestone Dec 5, 2014

caitp added the type: docs label Dec 5, 2014

jesselpalmer reviewed Dec 5, 2014
View reviewed changes

docs/app/assets/css/docs.css

.view-source {

margin-right:10px;

#sidenav-toc.md-locked-open[flex="33"] {

Copy link

Member

jesselpalmer Dec 5, 2014

Choose a reason for hiding this comment

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

Add return.

gkalpak force-pushed the materialize-2 branch from fb3e6bd to 3904cd5 Compare December 5, 2014 23:11

gkalpak force-pushed the materialize-2 branch from 52e65bf to 7a51d5d Compare December 7, 2014 11:01

caitp closed this Dec 9, 2014

gkalpak deleted the materialize-2 branch December 15, 2014 11:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(app): refactor the Docs App based on the MD prototype #10341

docs(app): refactor the Docs App based on the MD prototype #10341

gkalpak commented Dec 5, 2014

googlebot commented Dec 5, 2014

googlebot commented Dec 5, 2014

googlebot commented Dec 5, 2014

googlebot commented Dec 5, 2014

gkalpak commented Dec 5, 2014

petebacondarwin commented Dec 5, 2014

petebacondarwin Dec 5, 2014

gkalpak Dec 5, 2014

jesselpalmer Dec 5, 2014

gkalpak Dec 5, 2014

jesselpalmer Dec 6, 2014

jesselpalmer Dec 5, 2014

gkalpak commented Dec 5, 2014

caitp commented Dec 6, 2014

jesselpalmer commented Dec 6, 2014

gkalpak commented Dec 6, 2014

jesselpalmer commented Dec 6, 2014

caitp commented Dec 6, 2014

caitp commented Dec 6, 2014

gkalpak commented Dec 7, 2014

gkalpak commented Dec 8, 2014

caitp commented Dec 8, 2014

gkalpak commented Dec 8, 2014

caitp commented Dec 9, 2014

docs(app): refactor the Docs App based on the MD prototype #10341

docs(app): refactor the Docs App based on the MD prototype #10341

Conversation

gkalpak commented Dec 5, 2014

googlebot commented Dec 5, 2014

googlebot commented Dec 5, 2014

googlebot commented Dec 5, 2014

googlebot commented Dec 5, 2014

gkalpak commented Dec 5, 2014

petebacondarwin commented Dec 5, 2014

petebacondarwin Dec 5, 2014

Choose a reason for hiding this comment

gkalpak Dec 5, 2014

Choose a reason for hiding this comment

jesselpalmer Dec 5, 2014

Choose a reason for hiding this comment

gkalpak Dec 5, 2014

Choose a reason for hiding this comment

jesselpalmer Dec 6, 2014

Choose a reason for hiding this comment

jesselpalmer Dec 5, 2014

Choose a reason for hiding this comment

gkalpak commented Dec 5, 2014

caitp commented Dec 6, 2014

jesselpalmer commented Dec 6, 2014

gkalpak commented Dec 6, 2014

jesselpalmer commented Dec 6, 2014

caitp commented Dec 6, 2014

caitp commented Dec 6, 2014

gkalpak commented Dec 7, 2014

gkalpak commented Dec 8, 2014

caitp commented Dec 8, 2014

gkalpak commented Dec 8, 2014

caitp commented Dec 9, 2014