Restructure site #45

ashwini0529 · 2017-12-14T09:45:27Z

Fixes #44

Monal5031

Don't push .DS_Store and .idea files 😛

jayvdb · 2017-12-14T09:52:41Z

.idea/community.iml

@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8"?>


do we need all this .idea/ stuff?

Creating an issue for it (gitignore)

I don't think so. Editor added files I guess. :/

jayvdb · 2017-12-14T09:54:56Z

templates/activity.html

@@ -1,41 +0,0 @@
-<!doctype html>


please leave the activity/ area in place. it will grow. you can include the one activity widget on the front page to fill the white space, but it also needs to be on activity/

jayvdb

remove .DS_Store and can you load materialize from a CDN instead of putting it in the repo

Monal5031

Keep CSS separate from templates

Monal5031 · 2017-12-14T09:59:30Z

templates/index.html

+      <!--Import materialize.css-->
+      <link type="text/css" rel="stylesheet" href="../static/css/materialize.min.css"  media="screen,projection"/>
+        <style>
+        nav {


Please create a new style.css for this don't include CSS in here, we may need to add new things in future.

ashwini0529 · 2017-12-14T10:09:45Z

@Monal5031 done!
@jayvdb Done! 😄

jayvdb · 2017-12-14T10:14:19Z

Isn't coala allowed?

No. it is a generic community website which other orgs can fork and use without customisation. (see GCI task ).
We derive the org name from the repo name. This is already done for you by the build, and org_name.txt is created, and can be used.

Monal5031 · 2017-12-14T10:12:18Z

.gitignore

@@ -103,3 +103,4 @@ ENV/
 _site/
 /public/
 org_name.txt
+.idea


Separate issue for it is there #46 revert this change

Monal5031 · 2017-12-14T10:12:39Z

static/css/style.css

+    -moz-user-select: none;
+    -webkit-user-select: none;
+    -ms-user-select: none;
+}


newline please 😛

Monal5031 · 2017-12-14T10:13:50Z

static/css/style.css

@@ -0,0 +1,13 @@
+nav {


Oh sorry, just realised we already have a main.css we should keep this there. Sorry 😓

Monal5031 · 2017-12-14T10:15:41Z

@ashwini0529 .DS_Store is still there 😓 .

ashwini0529 · 2017-12-14T12:05:30Z

@jayvdb it would be a tough job to eliminate things like this. We can minimize the work by using a config file. That could contain several parameters like org_name, org_url, etc.
It would be then easy to access those parameters and use them. I'm struggling to access the org_name in footer.html :/

jayvdb · 2017-12-14T12:06:51Z

templates/activity.html

+
+    <nav>
+    <div class="nav-wrapper">
+      <a href="#" class="brand-logo">&nbsp;<img src="https://api.coala.io/en/latest/_static/images/coala_logo.svg"></a>


no. The objective was to solve #42 , fixing #1 .

So should i remove the logo? 😟

I've referred this issue in the commit message. Will that work?

jayvdb · 2017-12-14T12:08:35Z

templates/index.html

-    <li><a href="/activity/">GitHub activity</a>
-</ul>
-</body>
+{% include 'activity.html' %}


jayvdb · 2017-12-14T12:08:41Z

templates/footer.html

+            <a class="grey-text text-lighten-4 right" href="#!">GNU AGPL v3.0</a>
+        </div>
+    </div>
+</footer>


jayvdb · 2017-12-14T12:10:27Z

templates/activity.html

+      <link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">
+      <!--Import materialize.css-->
+      <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/materialize/0.100.2/css/materialize.min.css">
+      <link rel="stylesheet" href="../static/css/style.css">


../static does appear in the deployed website. use django-distill

jayvdb · 2017-12-14T12:14:57Z

We can minimize the work by using a config file. That could contain several parameters like org_name, org_url, etc.

No. This is an architecture choice. See the GCI task. This is supposed to be challenging.
org_name.txt is already a 'config file', generated so it can be easily accessed.

I'm struggling to access the org_name in footer.html :/

Please push your current code so we can take a look at how you've solved the other problems, and investigate how to solve this one.

ashwini0529 · 2017-12-16T06:15:24Z

@jayvdb the way to access org_name in this PR will be much easier. I think we should go ahead with this PR. What do you think? 😕

jayvdb · 2017-12-16T06:27:14Z

it may be easier, but it is wrong. You will understand why if you review my PR, and read the issues that it solves.

ashwini0529 · 2017-12-16T13:04:12Z

As now #48 is merged, I'd have to change few things up.
How do I proceed with it? @jayvdb

jayvdb · 2017-12-16T13:11:06Z

community/context_processors.py

@@ -0,0 +1,3 @@
+def get_org_name(request):
+    org_name = open('org_name.txt').readline()


This needs to be updated now that 'org_name.txt' is no longer generated.

gitmate-bot · 2017-12-16T18:45:26Z

community/context_processors.py

+
+def get_org_name(request):
+    org_name = get_owner()
+    print (org_name)


The code does not comply to PEP8.

Origin: PEP8Bear, Section: all.autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpkpr3iyck/community/context_processors.py +++ b/tmp/tmpkpr3iyck/community/context_processors.py @@ -3,5 +3,5 @@ def get_org_name(request): org_name = get_owner() - print (org_name) + print(org_name) return {'org_name': org_name}

gitmate-bot · 2017-12-16T18:45:27Z

community/context_processors.py

+
+def get_org_name(request):
+    org_name = get_owner()
+    print (org_name)


E211 whitespace before '('

Origin: PycodestyleBear (E211), Section: all.autopep8.

gitmate-bot · 2017-12-16T18:45:27Z

templates/footer.html

+                    </div>
+                    <div class="col m6 s12 center">
+                        <div class="gitter-btn center">
+                            <a href="https://gitter.im/coala" class="waves-effect waves-light btn-large"><i class="fa fa-commenting-o" aria-hidden="true"></i><span class="footer-btn-text">Join Chat On Gitter</span></a>


The line contains the keyword 'coala'.

Origin: KeywordBear, Section: generalization.

ashwini0529 · 2017-12-16T18:48:33Z

@jayvdb changes done! 😄

andrewda

Let's add an HTML linter bear too.

andrewda · 2017-12-16T18:51:33Z

templates/index.html

+<div class="section no-pad-bot" id="index-banner">
+
+        {% include 'issues_graph.html' %}
+</div>


Indentation seems weird here

andrewda · 2017-12-16T18:53:32Z

templates/twitter_feed.html

+        <div class="container">
+<a class="twitter-timeline" href="https://twitter.com/{{ org_twitter_handle }}">Tweets by {{ org_twitter_handle }}
+</a>
+<script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script></div></div>


All this indentation is really off. Closing divs should go on their own lines.

andrewda · 2017-12-16T18:54:14Z

twitter/view_twitter.py

-                     twitter_handle=org_twitter_handle))
+        return render(request, 'twitter_feed.html', context=org_data)
+    else:
+        return HttpResponse("Sorry, Organisation's twitter handle not found!")



Remove extra newline.

andrewda · 2017-12-16T18:56:47Z

templates/footer.html

+                <div class="row footer-btn-row">
+                    <div class="col m6 s12 center">
+                        <div class="twitter-btn center">
+                            <a href="https://twitter.com/{{ org_name }}_io" class="waves-effect waves-light btn-large"><i class="fa fa-twitter" aria-hidden="true"></i><span class="footer-btn-text">Follow Us On Twitter</span></a>


IMO doing stuff like this is completely against the point of not using the word coala as this will only work for us, not any other org. This might have already been discussed and will be fixed later, just wanted to provide my input on this.

Exactly. We’ll have to create a config file. But as discussed, we’ll come up with config file in next PR

jayvdb · 2017-12-17T08:46:20Z

templates/activity.html

+{% extends 'includes.html' %}
+{% block content %}
+    <div class="section no-pad-bot" id="index-banner">
+        <div class="container">


This div is all a repeat of what is on index.html ?

no. it generates code something like http://paste.ubuntu.com/26200076/

But why?
This page should have the standard header and footer and no extra stuff. It is just a page, like any other. The activity page is not the main page.

jayvdb · 2017-12-17T08:48:42Z

templates/index.html

+                    <div class="card" style="overflow: visible;">
+                        <div class="card-image waves-effect waves-block waves-light">
+                            <img class="activator"
+                                 src="https://api.{{ org_name }}.io/en/latest/_static/images/{{ org_name }}_logo.svg">


No, no, no. This is coala specific URLs.

What is more, we have actually already built a solution for generic logos. You only need to use it.

can you please tell me the usage? I was coming up with a proposal of introducing a config file to have all the org related information in my next PR.

@jayvdb should I use the header.png file?

No it shouldnt use header.png . I've created #55 to remove that.

Please stop consuming so much of other mentors time. It is counter productive. search this repo for "logo".

If your next iteration of this PR does not comply with the goals of this project, I am closing the PR and unassigning.

Closes coala#44

ashwini0529 · 2017-12-22T20:15:32Z

@jayvdb this would be the last major update that I would like to propose for this PR. Please have a look at the website.
If there's anything else that could be added or fixed, please let me know, or else you can close this PR. :)

blazeu · 2017-12-26T16:22:45Z

templates/activity.html

+{% block content %}
+    <div class="section no-pad-bot" id="index-banner">
+        <div class="container">
+            <h1 class="header center  col-test">Welcome</h1>


Double space

blazeu · 2017-12-26T16:24:54Z

templates/activity.html

-    <br>
+
+    {% include 'footer.html' %}
+    <!--Import jQuery before materialize.js-->


This is not needed,
and the comment is in wrong location.

blazeu · 2017-12-26T16:25:17Z

templates/includes.html

+    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/materialize/0.100.2/css/materialize.min.css">
+    <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css">
+    <link rel="stylesheet" href="../static/css/main.css">
+    <script type="text/javascript" src="https://code.jquery.com/jquery-3.2.1.min.js"></script>


jquery can stay in the bottom of the body.

blazeu · 2017-12-26T16:27:40Z

templates/index.html

+                $('#chartType').on('change', function () {
+                    updateChart($('#chartType').val())
+                });
+                updateChart($('#chartType').val());


This can be moved to charts.js so it's not duplicated in issues_graph page.

ashwini0529 · 2017-12-26T17:26:18Z

@jayvdb are we going to merge this PR? If we are, I'd be fixing the review that @blazeu gave.

jayvdb · 2018-01-02T10:59:34Z

This PR has conflicts :/

jayvdb · 2018-01-02T11:00:11Z

And there are review comments by @blazeu which havent been done.

jayvdb · 2018-01-02T11:03:08Z

templates/logs.html

+                    <li class="collection-item">{{ log }}<a href="#!" class="secondary-content">
+                        {% if '[ERROR]' in log %}
+                        <i class="fa fa-times-circle fa-2x"></i></a>
+                            {% elif '[WARNING]' in log %}


wrong indentation

jayvdb · 2018-01-02T11:03:16Z

templates/logs.html

+                            {% elif '[WARNING]' in log %}
+                            <i class="fa fa-exclamation-triangle fa-2x"></i></a>
+                        {% else %}
+                    <i class="fa fa-info-circle fa-2x"></i></a>


wrong indentation

jayvdb · 2018-01-02T11:10:50Z

templates/footer.html

+    <div class="footer-copyright">
+        <div class="container">
+            <span class="left">© 2017 {{ org_name }}</span>
+            <a class="grey-text text-lighten-4 right" href="https://github.com/{{ org_name }}"><i


hmm, how does "github.com{{ org_name }}" end up as github.com/coala ? django automatically adds the /? o_O

jayvdb · 2018-01-02T11:11:19Z

templates/footer.html

+            <span class="left">© 2017 {{ org_name }}</span>
+            <a class="grey-text text-lighten-4 right" href="https://github.com/{{ org_name }}"><i
+                    class="fa fa-github fa-2x"></i></a>&nbsp;&nbsp;
+            <a class="grey-text text-lighten-4 right" href="https://twitter.com/{{ org_name }}"><i


strange alignment of the <i...

jayvdb · 2018-01-02T11:11:55Z

templates/footer.html

+<footer class="page-footer">
+    <div class="footer-copyright">
+        <div class="container">
+            <span class="left">© 2017 {{ org_name }}</span>


2017 should be a generated value.

but more importantly, this implies something different than https://github.com/coala/community/blob/master/LICENSE says

jayvdb · 2018-01-02T11:44:36Z

templates/index.html

+            <h1 class="header center  col-test">Welcome</h1>
+            <div class="row center">
+                <h5 class="header col s12 light">Hello, world. You are at the {{ org_name }} community website.</h5>
+                <h4 class="header col s12 light">Meet awesome mentors</h4>


this is not how we would do a mentors app. please remove, and let that be achieved in the relevant issue.

jayvdb · 2018-01-02T11:46:15Z

templates/includes.html

+<nav>
+    <div class="nav-wrapper">
+        <a href="#" class="brand-logo">&nbsp;<img
+                src="{{ org_logo }}"></a>


org_logo isnt declared, and thus it does not appear on https://deploy-preview-45--coala-community.netlify.com/

It is currently working on http://coala-community.netlify.com/

jayvdb · 2018-01-02T11:47:16Z

templates/includes.html

+        <a href="#" class="brand-logo">&nbsp;<img
+                src="{{ org_logo }}"></a>
+        <ul id="nav-mobile" class="right hide-on-med-and-down">
+            <li><a href="/gci">GCI</a></li>


perhaps this list can be generated?

jayvdb · 2018-01-02T11:53:54Z

activity/views.py

+from django.shortcuts import render
+
+
+def index(request):


this duplicates information in community/urls.py . why do we need it here? (what are the benefits of this duplication?)

If we have it here, can we somehow avoid having it also in community/urls.py ? Maybe a feature request in django-distill?

jayvdb · 2018-01-02T11:56:38Z

community/settings.py

@@ -121,3 +121,5 @@
 MEDIA_URL = '/media/'
 MEDIA_ROOT = os.path.join(BASE_DIR, 'media')
 STATICFILES_DIRS = ['static/']
+TEMPLATES[0]['OPTIONS']['context_processors'].append(
+    'community.context_processors.get_org_name')


Move this into the TEMPLATES declaration.

gitmate-bot added size/XXL process/pending review labels Dec 14, 2017

Monal5031 suggested changes Dec 14, 2017

View reviewed changes

jayvdb reviewed Dec 14, 2017

View reviewed changes

ashwini0529 force-pushed the ashwini0529/revamp branch from ada99d1 to 6d0f65d Compare December 14, 2017 09:55

jayvdb requested changes Dec 14, 2017

View reviewed changes

Monal5031 suggested changes Dec 14, 2017

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Dec 14, 2017

ashwini0529 force-pushed the ashwini0529/revamp branch from 6d0f65d to e79ec05 Compare December 14, 2017 10:07

gitmate-bot added size/S process/pending review and removed size/XXL process/wip labels Dec 14, 2017

Monal5031 suggested changes Dec 14, 2017

View reviewed changes

jayvdb requested changes Dec 14, 2017

View reviewed changes

ashwini0529 force-pushed the ashwini0529/revamp branch from e79ec05 to ec600af Compare December 14, 2017 12:39

gitmate-bot added difficulty/medium initiatives/gci labels Dec 14, 2017

ashwini0529 force-pushed the ashwini0529/revamp branch 2 times, most recently from 813ae4d to 6ca750e Compare December 14, 2017 12:43

gitmate-bot added size/XL and removed size/S labels Dec 14, 2017

ashwini0529 force-pushed the ashwini0529/revamp branch from 6ca750e to 994c3cc Compare December 14, 2017 12:44

jayvdb reviewed Dec 16, 2017

View reviewed changes

ashwini0529 force-pushed the ashwini0529/revamp branch from 28bb4a1 to 5bc8864 Compare December 16, 2017 18:45

gitmate-bot reviewed Dec 16, 2017

View reviewed changes

ashwini0529 force-pushed the ashwini0529/revamp branch from 5bc8864 to e1e4b6b Compare December 16, 2017 18:47

andrewda suggested changes Dec 16, 2017

View reviewed changes

ashwini0529 force-pushed the ashwini0529/revamp branch from e1e4b6b to 41655e6 Compare December 17, 2017 08:40

jayvdb reviewed Dec 17, 2017

View reviewed changes

jayvdb requested changes Dec 17, 2017

View reviewed changes

jayvdb added process/wip and removed initiatives/gci process/pending review labels Dec 17, 2017

jayvdb mentioned this pull request Dec 19, 2017

Move logo on the front page #42

Closed

Restructure site and bug fixes

879517b

Closes coala#44

ashwini0529 force-pushed the ashwini0529/revamp branch from 41655e6 to 879517b Compare December 22, 2017 20:11

ashwini0529 mentioned this pull request Dec 22, 2017

Add 'Fork Me On GitHub' ribbons #24

Open

blazeu requested changes Dec 26, 2017

View reviewed changes

jayvdb requested changes Jan 2, 2018

View reviewed changes

gitmate-bot added the status/STALE label Jan 10, 2018

ashwini0529 closed this Mar 1, 2018

gitmate-bot assigned wisn Mar 4, 2018

		@@ -0,0 +1,3 @@
		def get_org_name(request):
		org_name = open('org_name.txt').readline()

Restructure site #45

Restructure site #45

Conversation

ashwini0529 commented Dec 14, 2017

Monal5031 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb left a comment

Choose a reason for hiding this comment

Monal5031 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashwini0529 commented Dec 14, 2017

jayvdb commented Dec 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monal5031 commented Dec 14, 2017

ashwini0529 commented Dec 14, 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

jayvdb commented Dec 14, 2017

ashwini0529 commented Dec 16, 2017

jayvdb commented Dec 16, 2017

ashwini0529 commented Dec 16, 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

ashwini0529 commented Dec 16, 2017

andrewda 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

ashwini0529 Dec 17, 2017 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashwini0529 commented Dec 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blazeu Dec 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashwini0529 commented Dec 26, 2017

jayvdb commented Jan 2, 2018

jayvdb commented Jan 2, 2018

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

jayvdb commented Dec 14, 2017 •

edited

Loading

ashwini0529 Dec 17, 2017 •

edited

Loading

blazeu Dec 26, 2017 •

edited

Loading