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

Fixing the python and js packaging #172

Merged
merged 2 commits into from
Mar 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
recursive-include panoramix/templates *
recursive-include panoramix/static *
recursive-exclude panoramix/static/assets/node_modules *
Copy link
Member Author

Choose a reason for hiding this comment

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

This excludes the thousands of files in that folder to ship with the python package

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha, and they're already included in the bundled js files so are totally unnecessary

recursive-include panoramix/static/assets/node_modules/font-awesome *
recursive-exclude panoramix/static/docs *
recursive-exclude tests *
recursive-include panoramix/data *
recursive-include panoramix/migrations *
1 change: 1 addition & 0 deletions panoramix/assets/javascripts/dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require('brace/mode/css');
require('brace/theme/crimson_editor');

require('./panoramix-select2.js');
require('../node_modules/gridster/dist/jquery.gridster.min.css');
require('../node_modules/gridster/dist/jquery.gridster.min.js');

var Dashboard = function (dashboardData) {
Expand Down
1 change: 0 additions & 1 deletion panoramix/assets/javascripts/explore.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ require('bootstrap');
require('./panoramix-select2.js');

require('../node_modules/bootstrap-toggle/js/bootstrap-toggle.min.js');
require('../vendor/select2.sortable.js');

// css
require('../vendor/pygments.css');
Expand Down
4 changes: 3 additions & 1 deletion panoramix/assets/javascripts/featured.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
var $ = window.$ = require('jquery');
var jQuery = window.jQuery = $;
var px = require('./modules/panoramix.js');

require('datatables');
require('datatables-bootstrap3-plugin');
require('../node_modules/datatables-bootstrap3-plugin/media/css/datatables-bootstrap3.css')
require('bootstrap');

$(document).ready(function () {
Expand Down
2 changes: 2 additions & 0 deletions panoramix/assets/javascripts/modules/panoramix.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ var $ = require('jquery');
var jQuery = $;
var d3 = require('d3');

require('../../stylesheets/panoramix.css');

// vis sources
var sourceMap = {
area: 'nvd3_vis.js',
Expand Down
3 changes: 3 additions & 0 deletions panoramix/assets/javascripts/panoramix-select2.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
require('../node_modules/select2/select2.css');
require('../node_modules/select2-bootstrap-css/select2-bootstrap.min.css');
require('../node_modules/jquery-ui/themes/base/jquery-ui.css')
require('select2');
require('../vendor/select2.sortable.js');
3 changes: 0 additions & 3 deletions panoramix/assets/stylesheets/panoramix.css
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,6 @@ img.loading {
font-size: 14px;
padding: 5px;
}
.dashboard div.gridster {
visibility: hidden
}
.dashboard div.slice_content {
width: 100%;
height: 100%;
Expand Down
7 changes: 6 additions & 1 deletion panoramix/assets/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,14 @@ var config = {
include: APP_DIR,
loader: "style-loader!css-loader"
},
/* for require('*.less') */
/* for css linking images */
{ test: /\.png$/, loader: "url-loader?limit=100000" },
{ test: /\.jpg$/, loader: "file-loader" },
{ test: /\.gif$/, loader: "file-loader" },
/* for font-awesome */
{ test: /\.woff(2)?(\?v=[0-9]\.[0-9]\.[0-9])?$/, loader: "url-loader?limit=10000&minetype=application/font-woff" },
Copy link
Member Author

Choose a reason for hiding this comment

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

this still doesn't work...

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure what you mean by that. does it still compile the .less / throw errors during npm run dev|prod? this was necessary because it's compiling all of the bootstrap variables, if that works it should be okay.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh I thought it was an attempt at getting font-awesome to work as the failure is around woff files and such...

{ test: /\.(ttf|eot|svg)(\?v=[0-9]\.[0-9]\.[0-9])?$/, loader: "file-loader" },
/* for require('*.less') */
{
test: /\.less$/,
include: APP_DIR,
Expand Down
6 changes: 1 addition & 5 deletions panoramix/templates/panoramix/basic.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@
</title>
{% block head_meta %}{% endblock %}
{% block head_css %}
<link rel="stylesheet" type="text/css" href="/static/assets/node_modules/jquery-ui/themes/base/minified/jquery-ui.min.css" />
<link rel="stylesheet" type="text/css" href="/static/assets/node_modules/font-awesome/css/font-awesome.min.css" />
<link rel="stylesheet" type="text/css" href="/static/assets/node_modules/select2/select2.css" />
<link rel="stylesheet" type="text/css" href="/static/assets/node_modules/select2/select2-bootstrap.css" />
<link rel="stylesheet" type="text/css" href="/static/assets/stylesheets/panoramix.css" />
<link rel="stylesheet" type="text/css" href="/static/assets/node_modules/font-awesome/css/font-awesome.min.css" />
{% endblock %}
{% block head_js %}
<script src="/static/assets/javascripts/dist/css-theme.entry.js"></script>
Expand Down
3 changes: 1 addition & 2 deletions panoramix/templates/panoramix/dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

{% block head_css %}
{{ super() }}
<link rel="stylesheet" href="{{ url_for('static', filename="assets/node_modules/gridster/dist/jquery.gridster.min.css") }}">
<style id="user_style" type="text/css">
{{ dashboard.css or '' }}
</style>
Expand Down Expand Up @@ -66,7 +65,7 @@ <h2>
</div>
</div>
</div>
<div class="gridster content_fluid">
<div class="gridster content_fluid" style="visibility: hidden;">
Copy link
Contributor

Choose a reason for hiding this comment

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

css rule instead of inline style! why is this necessary? and is visibility: hidden; what you want or display: none;?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well I had the visibility:hidden in panoramix.css but it would flicker, while the javascript .gridster call alters the css or classes the pre and post css changes would show for a fraction of a second.

When the visibility:hidden is in panoramix.css it flickers, but not when it's inline... Let's land this so I can get this release out and we can revisit later. Also the display:none option might be better but the jquery $.show() would animate while there's already a lot going on so I went for this approach instead.

I think the location of the js (head vs tail) might have something to do with it as well, maybe all of our entry scripts should be head since they contain css now. Anyhow, I'm merging and pushing a release as this works well and we can work out the details later.

<ul>
{% for slice in dashboard.slices %}
{% set pos = pos_dict.get(slice.id, {}) %}
Expand Down
5 changes: 0 additions & 5 deletions panoramix/templates/panoramix/featured.html
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ <h4>{{ dataset.table_name }}</h4>
</div>
{% endblock %}

{% block head_css %}
{{ super() }}
<link rel="stylesheet" type="text/css" href="/static/assets/node_modules/datatables-bootstrap3-plugin/media/css/datatables-bootstrap3.css" />
{% endblock %}

{% block tail_js %}
{{ super() }}
<script src="/static/assets/javascripts/dist/featured.entry.js"></script>
Expand Down
6 changes: 1 addition & 5 deletions setup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from setuptools import setup, find_packages

version = '0.7.1'
version = '0.8.0'

setup(
name='panoramix',
Expand All @@ -9,10 +9,6 @@
"and druid.io"),
version=version,
packages=find_packages(),
package_data={'': [
'panoramix/migrations/alembic.ini',
'panoramix/data/birth_names.csv.gz',
]},
include_package_data=True,
zip_safe=False,
scripts=['panoramix/bin/panoramix'],
Expand Down