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

Adds edit and view buttons #1905

Merged
merged 7 commits into from
Jan 30, 2017
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
1 change: 1 addition & 0 deletions notebook/notebookapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ def init_handlers(self, settings):
handlers.extend([(r"/login", settings['login_handler_class'])])
handlers.extend([(r"/logout", settings['logout_handler_class'])])
handlers.extend(load_handlers('files.handlers'))
handlers.extend(load_handlers('view.handlers'))
handlers.extend(load_handlers('notebook.handlers'))
handlers.extend(load_handlers('nbconvert.handlers'))
handlers.extend(load_handlers('bundler.handlers'))
Expand Down
44 changes: 41 additions & 3 deletions notebook/static/tree/js/notebooklist.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ define([
$('.download-button').click($.proxy(this.download_selected, this));
$('.shutdown-button').click($.proxy(this.shutdown_selected, this));
$('.duplicate-button').click($.proxy(this.duplicate_selected, this));
$('.view-button').click($.proxy(this.view_selected, this));
$('.edit-button').click($.proxy(this.edit_selected, this));
$('.delete-button').click($.proxy(this.delete_selected, this));

// Bind events for selection menu buttons.
Expand Down Expand Up @@ -558,9 +560,9 @@ define([
$('.rename-button').css('display', 'none');
}

// Move is visible iff at least one item is selected, and none of them
// Move is visible if at least one item is selected, and none of them
// are a running notebook.
if (selected.length >= 1 && !has_running_notebook) {
if (selected.length > 0 && !has_running_notebook) {
$('.move-button').css('display', 'inline-block');
} else {
$('.move-button').css('display', 'none');
Expand Down Expand Up @@ -597,6 +599,22 @@ define([
$('.delete-button').css('display', 'none');
}

// View is visible when an item is renderable or downloadable
if (selected.length > 0 && !has_directory && selected.every(function(el) {
return el.path.match(/html?|json|jpe?g|png|gif|tiff?|svg|bmp|ico|pdf|doc|xls/);
})) {
$('.view-button').css('display', 'inline-block');
} else {
$('.view-button').css('display', 'none');
}

// Edit is visible when an item is editable
if (selected.length > 0 && !has_directory) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to whitelist/blacklist files for editability? E.g. you can't open a png in the text editor.

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 could use a regex pattern like above to include/exclude specific extensions. I just opened a PNG in the text editor and it's pretty useless and harmless, so I'm neutral.

Copy link
Member

Choose a reason for hiding this comment

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

That was what I was thinking. :-) I'd probably go for a blacklist. Maybe there are too many types to be worth trying to list them all, though. Do we have mimetypes here? Maybe we could block edit on image/*, with an exception for image/svg+xml.

$('.edit-button').css('display', 'inline-block');
} else {
$('.edit-button').css('display', 'none');
}

// If all of the items are selected, show the selector as checked. If
// some of the items are selected, show it as checked. Otherwise,
// uncheck it.
Expand Down Expand Up @@ -887,7 +905,7 @@ define([

var item_path = that.selected[0].path;

window.open(utils.url_path_join(that.base_url, 'files', item_path) + '?download=1');
window.open(utils.url_path_join(that.base_url, 'files', utils.encode_uri_components(item_path)) + '?download=1', IPython._target);
};

NotebookList.prototype.delete_selected = function() {
Expand Down Expand Up @@ -937,6 +955,26 @@ define([
});
};

NotebookList.prototype.view_selected = function() {
var that = this;
that.selected.forEach(function(item) {
var item_path = utils.encode_uri_components(item.path);
// Handle HTML files differently
var item_type = item_path.endsWith('.html') ? 'view' : 'files';
window.open(utils.url_path_join(that.base_url, item_type, utils.encode_uri_components(item_path)), IPython._target);
});
};

NotebookList.prototype.edit_selected = function() {
var that = this;
that.selected.forEach(function(item) {
var item_path = utils.encode_uri_components(item.path);
// Handle ipynb files differently
var item_type = item_path.endsWith('.ipynb') ? 'notebooks' : 'edit';
window.open(utils.url_path_join(that.base_url, item_type, utils.encode_uri_components(item_path)), IPython._target);
});
};

NotebookList.prototype.duplicate_selected = function() {
var message;
var selected = this.selected.slice(); // Don't let that.selected change out from under us
Expand Down
2 changes: 2 additions & 0 deletions notebook/templates/tree.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
<button title="Move selected" class="move-button btn btn-default btn-xs">Move</button>
<button title="Download selected" class="download-button btn btn-default btn-xs">Download</button>
<button title="Shutdown selected notebook(s)" class="shutdown-button btn btn-default btn-xs btn-warning">Shutdown</button>
<button title="View selected" class="view-button btn btn-default btn-xs">View</button>
<button title="Edit selected" class="edit-button btn btn-default btn-xs">Edit</button>
<button title="Delete selected" class="delete-button btn btn-default btn-xs btn-danger"><i class="fa fa-trash"></i></button>
</div>
</div>
Expand Down
30 changes: 30 additions & 0 deletions notebook/templates/view.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<!DOCTYPE HTML>
<html>

<head>
<meta charset="utf-8">
<title>{{page_title}}</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
</head>

<body>
<style type="text/css">
html, body, #container {
height: 100%;
}
body, #container {
overflow: hidden;
margin: 0;
}
#iframe {
width: 100%;
height: 100%;
border: none;
}
</style>
<div id="container">
<iframe id="iframe" sandbox="allow-scripts" src="/files/{{file_path}}"></iframe>
</div>
</body>

</html>
11 changes: 10 additions & 1 deletion notebook/tests/test_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,16 @@ def test_download(self):
disposition = r.headers.get('Content-Disposition', '')
self.assertIn('attachment', disposition)
self.assertIn('filename="test.txt"', disposition)

def test_view_html(self):
nbdir = self.notebook_dir.name

html = '<div>Test test</div>'
with open(pjoin(nbdir, 'test.html'), 'w') as f:
f.write(html)

r = self.request('GET', 'view/test.html')
self.assertEqual(r.status_code, 200)

def test_old_files_redirect(self):
"""pre-2.0 'files/' prefixed links are properly redirected"""
Expand Down Expand Up @@ -145,4 +155,3 @@ def test_old_files_redirect(self):
r = self.request('GET', url)
self.assertEqual(r.status_code, 200)
self.assertEqual(r.text, prefix + '/f3')

Empty file added notebook/view/__init__.py
Empty file.
26 changes: 26 additions & 0 deletions notebook/view/handlers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#encoding: utf-8
"""Tornado handlers for viewing HTML files."""

# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

from tornado import web
from ..base.handlers import IPythonHandler, path_regex
from ..utils import url_escape

class ViewHandler(IPythonHandler):
"""Render HTML files within an iframe."""
@web.authenticated
def get(self, path):
path = path.strip('/')
if not self.contents_manager.file_exists(path):
raise web.HTTPError(404, u'File does not exist: %s' % path)

basename = path.rsplit('/', 1)[-1]
self.write(
self.render_template('view.html', file_path=url_escape(path), page_title=basename)
)

default_handlers = [
(r"/view%s" % path_regex, ViewHandler),
]