chore(API docs): update header of angular-patternfly API Docs site #556
Conversation
|
Hi, top left links seem to be cut off:
Also, the "Components" and "Contribute" links don't seem to go anywhere and do not change the 'selected' state from "Getting Started"
This PR does not seem complete, customer ready. I'd be hesitant to merge this. Can we add a '[WIP]' to the title of this PR and consider not merging into master but rather to a dev branch until everything is ready? Thanks, |
Gruntfile.js
Outdated
| styles: ['node_modules/datatables.net-dt/css/jquery.dataTables.css', | ||
| 'node_modules/patternfly/dist/css/patternfly.css', | ||
| 'node_modules/patternfly/dist/css/patternfly-additions.css', | ||
| 'http://rawgit.com/junezhang/showcase/master/dist/styles/patternfly-showcase.css', |
There was a problem hiding this comment.
Is there a patternfly based repo this could link to rather than an individuals repo?
There was a problem hiding this comment.
+1
Each repo should have it's own get started and contribute pages, specific with details and information for that particular repo. June has already populated the one for Angular 1 here: http://rawgit.com/junezhang/showcase/master/index.html
@jeff-phillips-18 @dtaylor113 does this information look up to date and correct? If so, @dabeng can we introduce this to the angular repo with this PR?
There was a problem hiding this comment.
The README file needs some work which will then need to be incorporated into this. :(
I am working on updating it.
|
I'm not sure how this PR works with PF Showcase? PF Showcase looks like this:
Whereas this PR/ngDoc looks like this:
Other than the CSS issues which @dabeng mentions, how do these integrate together? Are they supposed to look exactly the same? PF Showcase masthead title says "PatternFly Showcase", this PR/ngDoc masthead title is "PATTERNFLY ANGUALR" -not the same case/caps |
dabeng
changed the title
|
@dtaylor113 , maybe you need the following processing.
|
|
@dabeng , thanks! That did the trick!
There are less files here for angular-patternfly, you can add your styles to these, or create another less file. It should be included in this PR. Couple of questions/issues:
Thanks! |
|
@dabeng , extra credit if you can solve the 'ANGULAR Error' for the tab title :-)
|
|
Hi @dtaylor113 , thanks a lot for your improvement advices :)
|
dabeng
changed the title
|
Hi @dabeng, |
|
Hi @dtaylor113 , commit message is modified accordingly. |
grunt-ngdocs-index.tmpl
Outdated
| <span class="icon-bar"></span> | ||
| </button> | ||
| <div class="navbar-brand"> | ||
| <a id="logo" title="PatternFly Angular" href="#"> |
There was a problem hiding this comment.
This should be 'Angular Patternfly'
grunt-ngdocs-index.tmpl
Outdated
| <div class="navbar-collapse navbar-collapse-pf-site collapse"> | ||
| <ul id="menu-primary" class="nav navbar-nav navbar-right"> | ||
| <li> | ||
| <a href="get-startted">Get Started</a> |
There was a problem hiding this comment.
Misspelling. This is a broken link.
grunt-ngdocs-index.tmpl
Outdated
| <a href="#">Components</a> | ||
| </li> | ||
| <li> | ||
| <a href="contribute">Contribute</a> |
There was a problem hiding this comment.
This is a broken link.
grunt-ngdocs-index.tmpl
Outdated
| <a href="https://rawgit.com/patternfly/patternfly-react/gh-pages/index.html">PatternFly React</a> | ||
| </li> | ||
| <li> | ||
| <a href="http://www.patternfly.org/patternfly-css/">PatternFly CSS</a> |
There was a problem hiding this comment.
I'm not sure this is the correct link. It takes you to an inner patternfly org page which isn't formatted for stand alone.
There was a problem hiding this comment.
This link is the only valid link for patternfly css.
There was a problem hiding this comment.
That page doesn't look ready for prime time. Do we have an estimate on when it will be?
There was a problem hiding this comment.
@jeff-phillips-18 sorry, not catch your point about the prime time.
This showcase list is a window for user getting to know what we are addressing in the framework implementation like the below picture.
Even though some showcase projects are pretty new, like PatternFly-React, PatternFly-WebComponents, we also want user to see the progress, and welcome any contribution. How do you think about it? also here at @LHinson , want to hear any suggestions?
There was a problem hiding this comment.
+1 with @jeff-phillips-18. The CSS should be linked in the future but we aren't ready yet. It should be removed for now and we can add at a later date.
grunt-ngdocs-index.tmpl
Outdated
| <a href="">Menu</a> | ||
| <ul> | ||
| <li> | ||
| <a href="get-startted">Get Started</a> |
grunt-ngdocs-index.tmpl
Outdated
| </li> | ||
| <li class="dropdown"> | ||
| <a href="#" class="dropdown-toggle" data-toggle="dropdown"> | ||
| <i class="fa fa-th-large" aria-hidden="true"></i> |
There was a problem hiding this comment.
aria-hidden=true and should have an sr-only span for "Patternfly Showcases"
Gruntfile.js
Outdated
| ngdocs: { | ||
| options: { | ||
| title: 'Angular Patternfly Documentation', | ||
| title: 'ANGULAR', |
There was a problem hiding this comment.
This should be 'Angular Patternfly'
There was a problem hiding this comment.
Sorry Jeff, if do that, the heading will become "Patternfly - Angular Patternfly", Don't you feel it will be wired?
There was a problem hiding this comment.
It should end up as simply "Angular Patternfly", whatever that takes.
|
the showcase.less file should be in misc (which is not less ready) not in styles. Putting it in styles will add it to the distribution and these styles should NOT be in the distribution. |
|
Not sure that I believe fixing: is extra credit. It seems caused by this PR. |
|
@jeff-phillips-18, unfortunately not. I see "Error: Page Not Found" in our published ngDocs |
|
Hi @jeff-phillips-18 , I have moved showcase css code to folder misc and updated the details you mentioned above 😊 |
|
Also, I don't believe this is a feat. No reason to bump the release for this. IMO, this is a chore since there are not distribution changes. |
grunt-ngdocs-index.tmpl
Outdated
| <a href="https://patternfly-webcomponents.github.io/components.html">PatternFly-WebComponents</a> | ||
| </li> | ||
| <li> | ||
| <a href="https://rawgit.com/patternfly/patternfly-react/gh-pages/index.html">PatternFly React</a> |
There was a problem hiding this comment.
I see different formatting for the names
- Patternfly-ng
- Patternfly-WebComponents
- Patternfly React
- Patternfly CSS
These should either be the repo name:
- patternfly-ng
- patternfly-webcomponents
- patternfly-react
- patternfly-css
Or the titles:
- Patternfly NG
- Patternfly Web Components
- Patternfly React
- Patternfly CSS
dabeng
changed the title
|
Add this comment inline above but will add here too... The PatternFly PatternLab CSS should be linked in the future but we aren't ready yet. It should be removed for now and we can add at a later date. This is for the CSS.Next initiative. |
|
@dtaylor113 , please review again. |
|
Hi @dabeng, I notice a weird 'white-out' effect when I move the mouse pointer off of the app launcher icon:
|
|
Hi @dabeng, I don't know if GitHub is acting up at the moment but none of the links in the app. launcher seem to work:
Again, I don't know if GitHub is acting up or if these landing pages aren't quite done yet. |
|
@LHinson @jeff-phillips-18 Okay, got it. I will remove the css part from the list. |
|
@dtaylor113 Thanks for pointing the "white-out" bug, I've fixed this bug on here - http://rawgit.com/junezhang/showcase/master/index.html |
|
Hi @dtaylor113 @jeff-phillips-18 , I have updated PR according to your advices. Please review. FYI, @dtaylor113 , regarding external links, I got the following snapshot, they looks great. Maybe the issue you mentioned above stems from internet or network agent. FYI, @jeff-phillips-18 , I have removed all unready links from the navigation bar. |
grunt-ngdocs-index.tmpl
Outdated
| </button> | ||
| <div class="navbar-brand"> | ||
| <a id="logo" title="Angular Patternfly" href="#"> | ||
| PATTERNFLY-<span class="secondary-logo" ng-bind-template="<%= title %>"></span> |
There was a problem hiding this comment.
Do we need the all caps PATTERNFLY here? Is this the design across all the showcases?
There was a problem hiding this comment.
Yes @jeff-phillips-18 , this is the uniformed design across all the showcases.
There was a problem hiding this comment.
So are other components PATTERNFLY - Patternfly NG, and PATTERNFLY - Patternfly Web Components? Seems redundant.
There was a problem hiding this comment.
@jeff-phillips-18 It should be PATTERNFLY-NG, PATTERNFLY-REACT, PATTERNFLY-WEB COMPONENTS, PATTERNFLY-ANGULAR. so here <%= title %> the title should be ANGULAR.
There was a problem hiding this comment.
This repository has been around far too long to start calling it PATTERNFLY-ANGULAR. It has been known as Angular Patternfly and should remain that way.
There was a problem hiding this comment.
@jeff-phillips-18 yes, we discussed it before. sorry for forgetting it.
so here the name should be ANGULAR PATTERNFLY, does it make sense?
| <li class="dropdown"> | ||
| <a href="#" class="dropdown-toggle" data-toggle="dropdown"> | ||
| <i class="fa fa-th-large" aria-hidden="true"> | ||
| <span class="sr-only">PatternFly Showcases</span> |
There was a problem hiding this comment.
This should use pfApplicationLauncher.
grunt-ngdocs-index.tmpl
Outdated
| <a href="https://rawgit.com/patternfly/patternfly-ng/master-dist/dist-demo/#/">PatternFly NG</a> | ||
| </li> | ||
| <li> | ||
| <a href="https://patternfly-webcomponents.github.io/components.html">PatternFly WebComponents</a> |
There was a problem hiding this comment.
WebComponents should be "Web Components" to match the page it directs you to. The page it directs you to is titled simply "Web Components". Not sure who is responsible, but I would expect that page's title to be "Patternfly Web Components"
grunt-ngdocs-index.tmpl
Outdated
| <a href="https://rawgit.com/patternfly/patternfly-ng/master-dist/dist-demo/#/">PatternFly NG</a> | ||
| </li> | ||
| <li> | ||
| <a href="https://patternfly-webcomponents.github.io/components.html">PatternFly WebComponents</a> |
There was a problem hiding this comment.
WebComponents should be "Web Components"
|
Hi @jeff-phillips-18 , If I use directive pfApplicationLauncher, could you tell me which file I put the following code in? $scope.navigationItems = [
{
title: "PatternFly NG",
href: "https://rawgit.com/patternfly/patternfly-ng/master-dist/dist-demo/#/"
},
{
title: "PatternFly Web Components",
href: "https://patternfly-webcomponents.github.io/components.html"
},
{
title: "PatternFly React",
href: "https://rawgit.com/patternfly/patternfly-react/gh-pages/index.html"
}
];
$scope.label = ' ';
$scope.isDisabled = false;
$scope.isList = true;
$scope.hiddenIcons = false; |
|
You would just use the patternfly version, not necessarily the Angular directive. I'm not sure you could get our directives to work at this point in the ng-docs template. You could try if you like by adding a file in the misc directory as a controller for the navbar and put those settings in there (including this new js file in the 'scripts' portion of the ngdocs settings in the Gruntfile). Then setting the controller for the nav and using the directive. It would be interesting to get this working as it could lead to further changes to the page and more that we could control. Changing to the simple Patternfly version of the application launcher is probably plenty though as there isn't much need here for any additional logic in the controller. However, there may be some adjustments necessary as I'm not sure how well the application launcher will work outside of a PF horizontal or vertical nav bar. It should so you may need to file some issues and do some workarounds until it is fixed. This is a good test for its adaptability 😉 |
|
Hi @jeff-phillips-18 , I have applied patternfly application launcher in the new header. |
|
This is what I meant by needing some adjustments. The dropdown list is too far right (partially off screen), the arrow should be centered on the trigger. |
| .showcase header .navbar-utility .applauncher-pf .dropdown-menu { | ||
| margin-top: 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
@dabeng you missed one piece of css code here
.dropdown-kebab-pf .dropdown-menu.dropdown-menu-right::after,
.dropdown-kebab-pf .dropdown-menu.dropdown-menu-right::before {
right: 10px;
}
|
Hi @jeff-phillips-18 @junezhang , I have updated showcase.css and got the following snapshots: |
|
I'm not seeing this fix when I build/run. |
|
Sorry, what are the differences between the screenshots? |
|
@dtaylor113 I think the difference between the screenshots is in different browsers, what they are looking like. |
|
yes, remembered it, we discussed it before. so we will use ANGLUAR PATTERNFLY. does it make sense?
June
On Thu, Aug 31, 2017 at 8:43 PM +0800, "Jeffrey Phillips" <notifications@github.com> wrote:
@jeff-phillips-18 commented on this pull request.
In grunt-ngdocs-index.tmpl:
- <img class="pull-left" src="<%= image %>">
- <% if (imageLink) { %>
+<body class="showcase">
+ <header>
+ <nav class="navbar navbar-default navbar-fixed-top">
+ <div class="container-fluid">
+ <div class="navbar-header">
+ <button type="button" class="navbar-toggle" ng-click="isOpen = !isOpen">
+ <span class="sr-only">Toggle navigation</span>
+ <span class="icon-bar"></span>
+ <span class="icon-bar"></span>
+ <span class="icon-bar"></span>
+ </button>
+ <div class="navbar-brand">
+ <a id="logo" title="Angular Patternfly" href="#">
+ PATTERNFLY-<span class="secondary-logo" ng-bind-template="<%= title %>"></span>
This repository has been around far too long to start calling it PATTERNFLY-ANGULAR. It has been known as Angular Patternfly and should remain that way.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@jeff-phillips-18 , maybe you can clean up your browser cache or directly review the build/run results on this address -- https://rawgit.com/dabeng/angular-patternfly/new-header-dist/docs/index.html#/api |
|
Hi @dtaylor113 , I listed snapshots in order to demonstrate that patternfly application launcher menu has the same styles in different browers 😊 |
Hi @dabeng, ok, thanks for the explanation. |
5 participants
Description
This is the sub-task of PTNFLY-2467.
@dlabrecq @dtaylor113 @jeff-phillips-18 @bleathem @LHinson
Based on the PatternFly Showcase Design[1], I've created a PR for updating the header.
The demo link is here: https://rawgit.com/dabeng/angular-patternfly/new-header-dist/docs/index.html#/api