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.

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

Booting web UI in Material Design #1626

Merged
merged 21 commits into from Apr 17, 2015
Merged

Booting web UI in Material Design #1626

merged 21 commits into from Apr 17, 2015

Conversation

shanzi
Copy link
Contributor

@shanzi shanzi commented Apr 9, 2015

Here is the initial work to build a web UI in Material Design for Buildbot.

After some discussion with @tothandras , we believe our projects will not be conflict or entangled with each other at least during the early level of developing. I decided to build some base parts of the Web UI that won't concerns about displaying data from buildbotService first.

I opened a new folder named md_base and the new Web UI will fake itself as the old base so that master need not to be modified. The basic front end structure is generated by guanlecoja and I added some dependencies according to my own consideration. I will explain some decisions below about the code by now to draw early feedback so that I can make it right as soon as possible.

Before the explanation is a screenshot showing the appearance now in browser:
material-design-early

Some Explanations

As you can see in the code, I uses guanlecoja as the generator of this front end app. I added the dependencies of Angular Material directly in guanlecoja/config.coffee.

guanlecoja-ui is not added as a dependency because I found it is kind of depending on bootstrap which is the framework we'd like to abandon. So some services such as the side nav bar and notification for the old web UI has not been migrated to the new one yet. I have managed to use side nav bar provided by Angular Material to achieve current effect. Whether to add these services to new web UI or not needs further evaluation. Please give your advice about this.

As I have said in the proposal, SVG icons will be an important advantage of the new UI compared with using icon font. I have already built the basic structures to use SVG icons. The SVG files are placed at src/icons. Under current configuration, a gulp plugin will merge all these icons into a single file in order to reduce HTTP request times. We no longer need several font files that contains hundreds of unused icons. Now just add icon files that is needed and we can even modify the shape of icon easily.

Current SVG filles added are from ionicons which is released under MIT license. I think it is compatible to buildbot's license.

Router frameworks has not been decided by now. The old UI is using router frameworks provided angular.ui but so far the official ngRoute should be enough. @tothandras also recommended a ngNewRouter. This part needs more discussion too.

Some code convention problems should be settle down too. Especially the indention width for several language. So far, I write jade and less with indention width of 2 spaces and coffee script with indention width of 4 spaces.

This is a quite uncompleted start yet. I summed up some tasks to be done in one or two weeks.

TODOS

  • basic page layout
  • loading and displaying SVG icons
  • figuring out which router frameworks to use
  • making menu items in side nav bar customizable by config so that we can mix in plugins such as water fall (half finished. still need to add glMenuService)
  • highlighting selected menu item in the side nav
  • add test to sidenav directive (solve the problem of unexpected httpDataGet when getting icons)
  • using menu service to handle side nav

@@ -0,0 +1,40 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

not sure about the svg. I'd prefer to stick with a font like font-awesome, which is so much lighter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are using md-icon, which is a wonderful solutions for using SVG icons. All SVG files will be merged into a single file and Angular Material will cache it with templateCache.

The using of these icons is nearly the same as using an icon font. md-icon will handle everything and we no longer need to include at least 3 font files (.svg, .woff, .ttf) as well as the icons that are not used.

The compatibility is not a problem for our project. Every browsers that supports AngularJS have already support SVG.

So I insist using SVG icons.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I think I understand more. Its okay if the svg are concatenated.

What should be avoided is to include icons directly in git, we should rather use bower to fetch those icons in bower package.

This will need update of guanlecoja to support configuration of which icons we want in the bower section:

"material-design-icons":
version: "1.0.1"
files: [ "notification/svg/production/ic_system_update_48px.svg", "notification/svg/production/ic_more_48px.svg" ]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea.

On Fri, Apr 10, 2015 at 8:26 PM Pierre Tardy notifications@github.com
wrote:

In www/md_base/src/icons/gear-outline.svg
#1626 (comment):

@@ -0,0 +1,40 @@
+

Ok, I think I understand more. Its okay if the svg are concatenated.

What should be avoided is to include icons directly in git, we should
rather use bower to fetch those icons in bower package.

This will need update of guanlecoja to support configuration of which
icons we want in the bower section:

"material-design-icons":
version: "1.0.1"
files: [ "notification/svg/production/ic_system_update_48px.svg",
"notification/svg/production/ic_more_48px.svg" ]


Reply to this email directly or view it on GitHub
https://github.com/buildbot/buildbot/pull/1626/files#r28140177.

Copy link
Member

Choose a reason for hiding this comment

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

There is also http://zavoloklom.github.io/material-design-iconic-font/icons.html

Problem of using the official bower package for material-design-icons is that it downloads the full set of png version, which is a painful 50MB.

The full font is 50kb compressed, so I dont thing this is worth the pain of concatenating the svgs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Directly use font is OK.

But your choice will be limited to the font collection you have choose and
it won't be quite convenient to merge icons from different sets. There are
also some other shortcomings of icon font such as if the icon is small, it
will blur as the browser is optimizing them as a font. There are also many
problems about hinting. My preference for SVG icons mostly comes from this
article of CSS Tricks https://css-tricks.com/icon-fonts-vs-svg/ It
describes why we should use SVG.

I think if we are using ionicons(http://ionicons.com). The size of bower
won't be a problem. The repo contains no png icons.

I think if the number of icons we are using is not so many (less than 10
icons), put the icons directly in the repo is ok as long as the license
permit. And we might need to make a vector logo of Buildbot instead of
current bitmap icon in the future, then SVG will be a better choice.

On Fri, Apr 10, 2015 at 8:39 PM, Pierre Tardy notifications@github.com
wrote:

In www/md_base/src/icons/gear-outline.svg
#1626 (comment):

@@ -0,0 +1,40 @@
+

There is also
http://zavoloklom.github.io/material-design-iconic-font/icons.html

Problem of using the official bower package for material-design-icons is
that it downloads the full set of png version, which is a painful 50MB.

The full font is 50kb compressed, so I dont thing this is worth the pain
of concatenating the svgs.


Reply to this email directly or view it on GitHub
https://github.com/buildbot/buildbot/pull/1626/files#r28140864.

@tardyp
Copy link
Member

tardyp commented Apr 9, 2015

That is a good start!

I think this make sense to temporarily use the services from guanlecoja-ui. When the time comes, we make it more modular, and have a guanlecoja-ui-core guanlecoja-ui-bs guanlecoja-ui-md modules

Only directives have to be replaced, which are 30% only of guanlecoja-ui

You can create the set of generic directives inside a separated module in base_md

@tardyp
Copy link
Member

tardyp commented Apr 9, 2015

Also, about ngNewRouter, I am not so sure for now. The way ui-router is programable allows to create routabl plugins very easily, while doing the route config in the main controller like with ngNewRouter, will make the config different. I think there are plenty of work todo not to add another thing

@tothandras
Copy link
Member

Yes, sadly the new router is not production ready yet. I didn't know that.

On Thu, Apr 9, 2015, 18:58 Pierre Tardy notifications@github.com wrote:

Also, about ngNewRouter, I am not so sure for now. The way ui-router is
programable allows to create routabl plugins very easily, while doing the
route config in the main controller like with ngNewRouter, will make the
config different. I think there are plenty of work todo not to add another
thing


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

@sa2ajj
Copy link
Contributor

sa2ajj commented Apr 9, 2015

(and it needs a rebase)

@shanzi
Copy link
Contributor Author

shanzi commented Apr 10, 2015

Good. Will add ui-router to the project.

@@ -0,0 +1,45 @@
doctype html
html(ng-app="app", ng-controller="appController as app")
Copy link
Member

Choose a reason for hiding this comment

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

I am happy to see the controller as syntax. Don't forget to change the $scope.title to @title in the appController and to app.title in the view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, along with ui-route.

@shanzi
Copy link
Contributor Author

shanzi commented Apr 11, 2015

Latest appearance

highlight

nav item highlighting supported, sidenav is now a directive.

There is a problem when testing the directive as the mocks will regard the HTTP request for the SVG icons as a data request. Test for the directive will be pushed up once this problems is resolved.

@tothandras
Copy link
Member

Looks nice 👍

@tardyp
Copy link
Member

tardyp commented Apr 11, 2015

I think there are methods in order to tell httpbackend mock to use bypass mode if the url is *.svg
google is your friend.

@gsemet
Copy link

gsemet commented Apr 11, 2015

+1

@shanzi
Copy link
Contributor Author

shanzi commented Apr 13, 2015

I added an menu service similar to glMenuService to provide item managing function for Sidenav. Also added a proxy class to be compatible with plugins such as waterfall. The proxy class fake itself as glMenuService and pass the calling of addGroup to menuServiceProvider's addItem.

@tardyp The number of icons are using now is less than 10, so I think by now add svg directly in the repo is ok, you can regard them as something like image resources. we can turn to bower after the number of icons grows larger. After all. turning to bower will concerns modifying guanlecoja, that might not proper to be finished in this PR, so I removed this from TODO.

I think this pull request has already finished what it is intended to do and is ready to be merged. Please review the code and give comments.

@shanzi shanzi changed the title [WIP] Booting web UI in Material Design Booting web UI in Material Design Apr 13, 2015
@shanzi
Copy link
Contributor Author

shanzi commented Apr 14, 2015

@tardyp @dustin @sa2ajj
Any further comments? What should I do now? :)

@tardyp
Copy link
Member

tardyp commented Apr 14, 2015

Do you have a demo server that we can check?

@shanzi
Copy link
Contributor Author

shanzi commented Apr 14, 2015

@tardyp OKay, I will deploy one in one or two days

@tothandras
Copy link
Member

Here is my docker file I used: https://gist.github.com/tothandras/4f59bfdffcfa49b8f4ba
Hope it helps! :)

@shanzi
Copy link
Contributor Author

shanzi commented Apr 14, 2015

@tothandras cooool!
I'm now using dokku on my VPS, I'm considering to write a heroku compatible build pack for this :)

@sa2ajj
Copy link
Contributor

sa2ajj commented Apr 14, 2015

@shanzi I'd leave you in capable hands of @tardyp :) While I might have some comments about the look, I'd rather have the code right.

@shanzi
Copy link
Contributor Author

shanzi commented Apr 16, 2015

@tardyp The live demo is here http://buildbot.apps.io-meter.com/

@dustin @sa2ajj The build is failing because of a network error during pip install Six. Please trigger a rebuilding :)

BTW: I have come up with a build pack that is extremely easy to use for deploying Buildbot's master on Heroku or Dokku with which I deployed my live demo on my VPS. The only required thing for the users is a single .env file to specify the build pack. One can also specify the git repo and branch to install Buildbot from. I will update the build pack in the future for more flexible usage.

For details, please refer to https://github.com/shanzi/buildbot-master-buildpack :)

cc @tothandras

@sa2ajj
Copy link
Contributor

sa2ajj commented Apr 16, 2015

@shanzi very impressive.

@djmitche I think we could add this buildbot-buildpack into github/buildbot hierarchy, if @shanzi does not object. (and add a couple of words to the docs about it.)

@shanzi
Copy link
Contributor Author

shanzi commented Apr 16, 2015

I of course not object to add this to github/buildbot! :)

There are still many improvements should be made to this buildpack. Also we
need a buildbot-slave-buildpack too. I will handle these in my spare time
and once it becomes stable, I will add some docs about it.

On Thu, Apr 16, 2015 at 12:07 PM, Mikhail Sobolev notifications@github.com
wrote:

@shanzi https://github.com/shanzi very impressive.

@djmitche https://github.com/djmitche I think we could add this
buildbot-buildpack into github/buildbot hierarchy, if @shanzi
https://github.com/shanzi does not object. (and add a couple of words
to the docs about it.)


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

@tothandras
Copy link
Member

The menu disappears on narrower screens (<960px).

@shanzi
Copy link
Contributor Author

shanzi commented Apr 16, 2015

@tothandras fixed it with a nav button, when the sidenav is hidden, you can click the nav button to show it.

@tardyp
Copy link
Member

tardyp commented Apr 16, 2015

Buildpacks have to be in their own git repository. But we can host it under
the buildbot group.

Also be aware that buildbot is not paas compliant yet because of the slave
port that needs to be on a different port as the web ui. This might work on
dokku because it is single host but won't on deis or cloud foundry. That is
one of the reason why I started my wamp poc.

Le jeu. 16 avr. 2015 06:12, Chase Zhang notifications@github.com a écrit :

@tothandras https://github.com/tothandras fixed it with a nav button,
when the sidenav is hidden, you can click the nav button to show it.


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

@sa2ajj
Copy link
Contributor

sa2ajj commented Apr 17, 2015

@tardyp
Copy link
Member

tardyp commented Apr 17, 2015

alright, I am gonna merge this as is.

I'd like you work at some point on getting the svgs from bower, but this can be in subsequent PR.

good work!

tardyp added a commit that referenced this pull request Apr 17, 2015
Booting web UI in Material Design
@tardyp tardyp merged commit 66374ce into buildbot:master Apr 17, 2015
@shanzi
Copy link
Contributor Author

shanzi commented Apr 18, 2015

Cool!

So glad that you like it. I will start to work on new tasks a couple days
later after make a little improvement on the build pack.

🍺
On Sat, Apr 18, 2015 at 3:01 AM Pierre Tardy notifications@github.com
wrote:

Merged #1626 #1626.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants