Reorganize static file structure and display revision data to user #16

purelogiq · 2017-06-02T20:40:55Z

Update: I've split the patch into smaller commits that have self contained commits.

See the "Document changes" patch with a lot of readme information.

To test this patch, you will have to start the lando-api docker image then in this projects docker-compose file edit the LANDO_API_URL to point to the docker-ip address:port of lando api.

It will help greatly to look at the structure of this PR in your file editor for it to make more sense.

============================================================
[wip] I will update the screenshot as I push more changes.

purelogiq · 2017-06-05T21:23:31Z

New WIP SS. Nothing special, but, all the data is being pulled in including parent revisions.

darkwing · 2017-06-13T15:25:54Z

Looks good so far. What is THIS_WONT_BE_NEEDED_LATER_API_KEY and why wont it be needed.

Back over to you

purelogiq · 2017-06-20T14:47:26Z

The last commit adds support for layouts and fixes some issues with how I was using Semantic UI. I will fully explain in the PR section when finished!

Also THIS_WONT_BE_NEEDED_LATER_API_KEY was a temporary fix for mozilla-conduit/lando-api#6 :D

purelogiq · 2017-06-22T08:47:07Z

purelogiq · 2017-06-22T08:49:15Z

Still haven't actually styled the revision page, just did all behind the scenes work to include sass and materialize. I've written a small novel in READMEs so please do check them out :D (there are 4 or so in total in this PR).

BTW the final PR will be more organized in terms of commits and so on. Getting there!

purelogiq · 2017-06-27T13:07:16Z

darkwing · 2017-06-27T20:21:15Z

landoui/static/src/css/components/Footer.scss

@@ -0,0 +1,3 @@
+.Footer {


Let's keep selectors as lowercase, especially if we're using - to separate selector words instead of camelcase.

Did you see the static/src/css/README.md ? They start with uppercase letters because of the SUIT CSS pattern discussed there (meant to allow us to have components for all our stuff, sorta like we did in React).

darkwing · 2017-06-27T20:22:43Z

landoui/static/src/js/lando.js

@@ -0,0 +1,2 @@
+// Setup one global namespace to hold all our classes.
+var Lando = Lando || {};


You can run into a runtime error with this; instead put window in:

window.Lando = window.Lando || {};

darkwing · 2017-06-27T20:26:07Z

landoui/static/src/css/colors.scss

@@ -0,0 +1,13 @@
+// In dev, you must make a change in lando.scss for changes here to be detected.
+// This file isn't autowatched by the Flask asset pipeline.


Can we do something about this? Initiate a watcher of some kind?

darkwing · 2017-06-27T20:31:19Z

landoui/app.py

@@ -73,8 +69,10 @@ def create_app(

    # Register routes via Flask Blueprints
    from landoui.pages import pages


Out of curiosity: why are we pulling these in here instead of at the top.

Not really not that I think of it :)

Yes actually! It is because these depend on the global oidc variable we set at the top.

darkwing · 2017-06-27T20:36:14Z

landoui/static/src/js/pages/RevisionPage.js

@@ -0,0 +1,33 @@
+/* Simple class to manage events and state on the /revision/{id} page */
+Lando.RevisionPage = class {


In looking at how this is used, we should really use the jQuery plugin pattern. You can find an example I created for MDN here:

https://github.com/mozilla/kuma/blob/master/kuma/static/js/moz-jquery-plugins.js#L287

Do you mean we should create 1 plugin for the entire 1 "RevisionPage" or a plugin for the collapsible that I made?

Got it I'll update this to use the pattern for the whole page e.g.$page.landoRevisionPage().
Also doing so will probably remove the need for the global Lando variable I made (maybe it wouldn't hurt to keep it)

darkwing · 2017-06-27T20:38:14Z

This isn't building properly:

Requirement already satisfied: setuptools>=11.3 in /usr/local/lib/python3.5/site-packages (from cryptography==1.8.1->-r /requirements.txt (line 55))
In --require-hashes mode, all requirements must have their versions pinned with ==. These do not:
    six from https://pypi.python.org/packages/c8/0a/b6723e1bc4c516cb687841499455a8505b44607ab535be01091c0f24f079/six-1.10.0-py2.py3-none-any.whl#md5=3ab558cf5d4f7a72611d59a81a315dc8 (from oic==0.9.1.0->-r /requirements.txt (line 27))
    packaging from https://pypi.python.org/packages/87/1b/c39b7c65b5612812b83d6cab7ef2885eac9f6beb0b7b8a7071a186aea3b1/packaging-16.8-py2.py3-none-any.whl#md5=c7326351bf015fa53c74b0075923ab02 (from cryptography==1.8.1->-r /requirements.txt (line 55))
ERROR: Service 'lando-ui' failed to build: The command '/bin/sh -c pip install -r /requirements.txt' returned a non-zero code: 1
...

darkwing · 2017-06-30T21:00:39Z

landoui/assets_src/js/README.md

+  }
+
+  bindEvents(){
+    $('button').click(clickHandler);


We should use .on('click', clickHandler)

darkwing · 2017-06-30T21:01:34Z

landoui/assets_src/js/README.md

+### Example Class for the widget above
+
+```javascript
+Lando.Widget = class {


Actually, is any of this documentation current?

darkwing · 2017-06-30T21:02:57Z

landoui/assets_src/js/lando.js

@@ -0,0 +1,2 @@
+// Setup one global namespace to hold all our classes.
+window.Lando = window.Lando || {};


No longer needed.

darkwing · 2017-06-30T21:03:26Z

landoui/assets_src/js/pages/RevisionPage.js

+  function RevisionPageCollapsible(){
+    let $revision = $(this).closest('.RevisionPage-parentRevision').first();
+
+    $(this).click(function(){


.on('click', fn)

darkwing · 2017-06-30T21:04:17Z

landoui/assets_src/js/pages/RevisionPage.js

+    $page.find('.RevisionPage-collapseLink').landoRevisionPageCollapsible();
+  }
+
+  return this.each(RevisionPage);


RevisionPage is so small we can probably just inline it. Also, only classes should be capitalized.

Shouldn't it be $(this).each()?

this.each I believe

darkwing · 2017-06-30T21:05:11Z

landoui/revisions.py

+
+@revisions.route('/revisions/<revision_id>')
+def get_revision(revision_id):
+    revision_api_url = os.getenv('LANDO_API_URL') + '/revisions/' + revision_id


Should we do:

{API_URL}/revisions/{REVISION_ID}'.format(...)?

darkwing · 2017-06-30T21:38:11Z

landoui/assets_src/js/pages/RevisionPage.js

+  const iconClassSelectors = '.fa-chevron-circle-right, .fa-chevron-circle-down';
+  const iconClassToggle = 'fa-chevron-circle-right, fa-chevron-circle-down';
+
+  return this.each(function(){


$(this).each?

pretty sure it is this.each, since this already refers to the jquery selector.

(and it works so far)

zalun

Ready to go after nits

zalun · 2017-07-05T14:42:28Z

landoui/assets_src/css/pages/RevisionPage.scss

+    display: none;
+    margin-top: .5rem;
+  }
+


Nit: empty lines, also later at the endo of the file

zalun · 2017-07-05T14:42:47Z

landoui/assets_src/css/components/Navbar.scss

+    color: $blue;
+  }
+}
+


Nit: empty lines

zalun · 2017-07-05T14:44:59Z

landoui/revisions.py

+@revisions.route('/revisions/<revision_id>')
+def get_revision(revision_id):
+    revision_api_url = '{}/revisions/{}'\
+        .format(os.getenv('LANDO_API_URL'), revision_id)


Nit: wouldn't it be better to write it like this?

'{}/revisions/{}'.format( os.getenv('LANDO_API_URL'), revision_id )

purelogiq · 2017-07-05T15:28:54Z

Fixed, gonna merge it :)

purelogiq force-pushed the plq/display-revision branch from a22b53b to c8444c7 Compare June 5, 2017 21:22

purelogiq force-pushed the plq/display-revision branch from f270b1a to 10aab82 Compare June 22, 2017 08:46

purelogiq force-pushed the plq/display-revision branch from c2e902a to d71264a Compare June 27, 2017 18:12

purelogiq changed the title ~~[WIP] Display revision data to user~~ Reorganize static file structure and display revision data to user Jun 27, 2017

darkwing suggested changes Jun 27, 2017

View reviewed changes

purelogiq force-pushed the plq/display-revision branch 3 times, most recently from b48e03b to ad186ca Compare June 29, 2017 23:12

purelogiq added 2 commits June 30, 2017 14:18

Vendor bulma, jquery, font-awesome, and delete unneed files

84e17c2

Add landoui logo

8eab568

purelogiq force-pushed the plq/display-revision branch 3 times, most recently from ff7be65 to 7828cc1 Compare June 30, 2017 20:52

darkwing suggested changes Jun 30, 2017

View reviewed changes

purelogiq force-pushed the plq/display-revision branch from 7828cc1 to 46585fe Compare June 30, 2017 21:28

darkwing suggested changes Jun 30, 2017

View reviewed changes

purelogiq requested a review from zalun July 5, 2017 14:12

zalun approved these changes Jul 5, 2017

View reviewed changes

purelogiq force-pushed the plq/display-revision branch from 46585fe to 280b317 Compare July 5, 2017 15:26

purelogiq added 2 commits July 5, 2017 11:27

Refactor static ui structure and display revisions

82721bf

Document changes with new static asset structure

deb69b8

purelogiq force-pushed the plq/display-revision branch from 280b317 to deb69b8 Compare July 5, 2017 15:27

purelogiq merged commit 8a5d3c3 into master Jul 5, 2017

purelogiq mentioned this pull request Jul 5, 2017

Update static asset documentation #20

Merged

purelogiq deleted the plq/display-revision branch September 26, 2017 15:13

		@@ -0,0 +1,2 @@
		// Setup one global namespace to hold all our classes.
		var Lando = Lando \|\| {};

		@@ -0,0 +1,13 @@
		// In dev, you must make a change in lando.scss for changes here to be detected.
		// This file isn't autowatched by the Flask asset pipeline.

		@@ -73,8 +69,10 @@ def create_app(

		# Register routes via Flask Blueprints
		from landoui.pages import pages

		@@ -0,0 +1,33 @@
		/* Simple class to manage events and state on the /revision/{id} page */
		Lando.RevisionPage = class {

		@@ -0,0 +1,2 @@
		// Setup one global namespace to hold all our classes.
		window.Lando = window.Lando \|\| {};

Reorganize static file structure and display revision data to user #16

Reorganize static file structure and display revision data to user #16

Conversation

purelogiq commented Jun 2, 2017 • edited Loading

purelogiq commented Jun 5, 2017

darkwing commented Jun 13, 2017

purelogiq commented Jun 20, 2017

purelogiq commented Jun 22, 2017

purelogiq commented Jun 22, 2017

purelogiq commented Jun 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darkwing commented Jun 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zalun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purelogiq commented Jul 5, 2017

purelogiq commented Jun 2, 2017 •

edited

Loading