Restructure templates to idiomatic Jinja

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

By idiomatic Jinja, I mean

• Inheritance instead of inclusion

• Arguments required are only the needed models to produce the required view [1]

This will make editing the templates easier, and will allow for a more consistent interface (by specifying base templates for each section).

[1] Model-View-Controller

CC: @qed777 @williamstein

Component: notebook

Author: Tim Dumol, Mitesh Patel

Reviewer: Mitesh Patel

Merged: sagenb-0.6

Issue created by migration from https://trac.sagemath.org/ticket/7786

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

Attachment: trac_7783-sage-scripts.patch.gz

Preliminary work. Worksheet pages not yet done.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

Attachment: trac_7786-template-jinja-idiomatic.patch.gz

Converts template structure to an inheritance based tree. Apply this patch alone to sagenb repo.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

comment:1

This is a big patch that makes the template structure more consistent by using inheritance. It also removes a lot of non-semantic markup (
,

---

, etc.). Please note that there are some visual changes (font stack, bar below banner at login.html, headers instead of bold tags at login.html, etc.). I hope they're not too major. Kindly note any changes that are too big and may require consultation at the mailing list.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

comment:2

Note: Depends on #7269, #7650, and dependencies.

[e14f4152-4982-4ace-8c95-73a0599b109b]

Author: Tim Dumol

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:3

I can only look at the patch right now, but I think these are much-needed and appreciated changes to SageNB. I think we should make merging this ticket (plus its dependencies) a priority. I'll try to start reviewing the patch soon.

As the merge nears, I'll rebase #7666 and its descendants, as necessary.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

Attachment: trac_7786-template-jinja-idiomatic.2.patch.gz

Rebased on #7650 and its dependencies. Apply this patch alone to sagenb repo.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

Rebased on #7650 and its dependencies. Apply this patch alone to sagenb repo.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

Attachment: trac_7786-template-jinja-idiomatic.2,patch.gz

Attachment: trac_7786-template-jinja-idiomatic.3.patch.gz

Updates source_code.html and adds styling for it. Also adds a warning to the .css files.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

Attachment: trac_7786-template-jinja-idiomatic.4.patch.gz

Adds missing div_wrap argument to template call at Cell.html()

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:4

V4 applies cleanly to 4.3.1.alpha0 + #7650's latest. With this configuration, I see

• Se test errors:

  • 4050: Exception: Timed out after 30000ms
  • 7433: Exception: ERROR: Element link=Sign out not found
  • 3960, 7433, 7428, 7444, searching_for_worksheets: Exception: ERROR: Element link=Worksheet not found

• Doctests:

  • Failures: challenge.py, template.py, worksheet.py.
  • Memory-leak(?): cell.py.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:5

• "Home" page contains "Toggle" link.
  • Printed/published worksheets: Click bar is visible (on hover). Clicking adds a cell, which appears to be editable.
  • Upon opening a worksheet: all existing input cells appear to have the same size.
  • Docbrowser worksheets:
  • Extra clickbar at top.
  • Shift-click is not disabled.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:6

• Minor: Message at top of main.sass should also refer to the readme.txt (cf. test_report.sass.
  • Just Se test 4050 fails with

trac_7843-cell_listdir.patch
trac_7844-notebook_address.patch
trac_7847-empty_trash_ie_ff.patch
trac_7846-no_CODE_PY_symlinks.patch
trac_7650-sagenb_doctesting_v3.patch
trac_7786-template-jinja-idiomatic.4.patch

!

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:7

• Actually, 4050 gives the same error (i.e., not a failure) as above.
  • js.py does not compress main.js.
  • 'Error:' --> 'Error: ' on the Sign in page.
  • Ratings:
  • Unrated worksheets are rated -1.0, it seems.
  • A worksheet's ratings page shows "rating[0]" under User and "rating[1]" under "Rating."
  • Out of curiosity: Is there a maximum comment length?

[e14f4152-4982-4ace-8c95-73a0599b109b]

Simple Jinja template dependency graph worksheet. Not a patch.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:8

Attachment: jinja_template_deps.html.gz

The attached worksheet (just paste it into an "Edit" window) generates a template dependency graph in Sage. I'm sure there are many improvements to make, but I hope it's a useful start. By the way, the last non-empty cell requires Graphviz's dot.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:9

• It seems there are two account_recovery.html pages.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:10

Actually, Graph.plot's partition option may be a better choice for coloring vertices. Anyway, have fun!

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:11

Hi Tim -- Do you mind if I make a separate, commuting patch here of just changes under sagenb/testing? If it's OK, I'd like to incorporate some changes I made at #7666, including running a test(s) by name (e.g., rt.run_any(['4088', 'test_3711'], verbosity=1)) and making it easier to test other browsers (e.g., first steps toward enabling Selenium Grid). Actually getting the tests to pass in the other browsers --- I found this very tedious / time-consuming --- we can leave for another ticket.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

comment:12

Sure, no problem.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

comment:13

By the way, the dependency graph is awesome. You may want to put it in the wiki somewhere, for Sage Notebook development tools.

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

Attachment: trac_7786-template-jinja-idiomatic.5.patch.gz

Fixes the issues pointed out (Se failures, etc.)

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

comment:14

The doctest failures are not yet fixed. I am having trouble with the Se tests: after a recent kernel upgrade, it seems that shutil.rmdir() fails silently, and os.path.exists reports the directory as gone, but is actually still there. I can only think of a kludgy fix of sleeping a while and looping until it can be deleted. Any ideas?

[e14f4152-4982-4ace-8c95-73a0599b109b]

Attachment: trac_7786-template-jinja-idiomatic.6.patch.gz

New test options. Should fix Se + doc tests. Replaces previous.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:15

V6 might help. I've tested just FF 3.5.6 on Linux, so far. Examples:

import sagenb.testing.run_tests as rt

# Selenium.
rt.setup_tests(environment='*firefox3 /usr/lib64/firefox-3.5.6/firefox')
rt.run_any()

# Selenium Grid.
envs = [
    '*firefox',
#    '*googlechrome',
    '*iexplore',
    '*opera',
#    '*safari'
    ]

for e in envs:
    rt.setup_tests('192.168.50.99', False, e)
    name = 'report_' + e.split()[0][1:] + '.html'
    rt.run_any(make_report=True, report_filename=name)

For other tickets:

• Parallel testing.
• Abstract away all self.selenium calls from sagenb/tests/*, i.e., put them all in a notebook_test_case.SeleniumTestCase. Then we may be able to reuse test_* for pure zope.testbrowser tests --- "just" write a corresponding notebook_test_case.ZTTestCase.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:16

Of course, please feel free to make changes!

On the dependency graph: Thanks! Maybe we should include parts of the code (wrapped in a try-except block) in template.py?

[e14f4152-4982-4ace-8c95-73a0599b109b]

Attachment: trac_7786-template-jinja-idiomatic.7.patch.gz

Conditional main.js compression. See js.py. Replaces previous.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:17

• It's not possible to delete compute cells from docbrowser worksheets, it seems.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:18

Reminder: Use self-closing <input /> tags in HTML files. WebKit browsers will complain about extra </input>s.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:19

Can we delete twist.Reset_css?

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

comment:20

Replying to @qed777:

Of course, please feel free to make changes!

On the dependency graph: Thanks! Maybe we should include parts of the code (wrapped in a try-except block) in template.py?

It doesn't sound like it can be used by a non-developer, so I don't think it's worth including into the package. That's just my opinion though.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:21

Should NotebookSettingsPage.render return a HTMLResponse, instead of a Response?

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

comment:22

Replying to @qed777:

Can we delete twist.Reset_css?

Yes, we can.

Should NotebookSettingsPage.render return a HTMLResponse, instead of a Response?

Yes.

But I think this patch has way too many changes in it as is. The clean up on twist.py should probably be spun off to another ticket.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:23

Replying to @TimDumol:

But I think this patch has way too many changes in it as is. The clean up on twist.py should probably be spun off to another ticket.

Sounds good. I need to do this anyway for public / remote interacts.

[e14f4152-4982-4ace-8c95-73a0599b109b]

Attachment: trac_7786-template-jinja-idiomatic.8.patch.gz

Further small fixes. Replaces previous.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:24

V8 should cover the problems above, except for deleting docbrowser cells, which is a known "problem" --- we don't save changes to these worksheets. Anyway, to the extent it counts, my review is positive, except:

• Is _topbar.sass missing? When compiling, I see

Sass::SyntaxError on line 29 of /.../sass/src/main.sass: File to import not found or unreadable: topbar.sass.

If I comment out the @import, the generated main.css seems to be missing topbar directives.

By the way, I removed template_error.html and base_popup.html, since they don't appear to be used. I also put set div_wrap = true in cell.html (when printing). I apologize for going too far, in case I have.

[e14f4152-4982-4ace-8c95-73a0599b109b]

Reviewer: Mitesh Patel

[e14f4152-4982-4ace-8c95-73a0599b109b]

Changed author from Tim Dumol to Tim Dumol, Mitesh Patel

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

Adds missing file _topbar.sass

[6bf4345a-8fdd-4a0e-b9a5-03107e609570]

comment:26

Attachment: trac_7786-template-jinja-idiomatic.9.patch.gz

Yes, it was missing. This patch fixes that problem.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:27

Positive review from me. If you agree, please update the status.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:28

Actually, I just noticed some problems with $(document).ready(). I'm working on V10.

[e14f4152-4982-4ace-8c95-73a0599b109b]

Attachment: trac_7786-template-jinja-idiomatic.10.patch.gz

DOM ready / load event timing fixes. Replaces previous.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:29

In V10, I replaced $(document).ready() in a few places with either synchronous evaluation or $(window).load() (in particular, $(document).load() does not always work). The main reason is timing --- the "DOM ready" event can fire too early for certain notebook initializations. For example, evaluate

import time
time.sleep(20)
print 'foo'

and reload the worksheet. Published interacts are another, forthcoming example...

I noticed that Worksheet.html_cell_list in V9

• Referred to a non-existent template published_worksheet.html.
• Is really no longer used. The lone remaining call, in twist.py sends refreshed HTML that notebook_lib.js ignores.

I've replaced the call with an empty string. However, the main problem is that HTML for published worksheets is now no longer cached by the server. Or am I mistaken?

I'll try to fix this in V11...

[e14f4152-4982-4ace-8c95-73a0599b109b]

Attachment: trac_7786-template-jinja-idiomatic.11.patch.gz

Cache HTML for published worksheets. Replaces previous.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:30

V11 is attached.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:31

If I save $\alpha$ in a text cell, then edit the cell again, I see Ë in the editor. Moreover, the HTML source now contains <span> tags, etc.

[e14f4152-4982-4ace-8c95-73a0599b109b]

Fix text cell typesetting. Replaces previous.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:32

Attachment: trac_7786-template-jinja-idiomatic.12.patch.gz

Replying to @qed777:

If I save $\alpha$ in a text cell, then edit the cell again, I see Ë in the editor. Moreover, the HTML source now contains <span> tags, etc.

V12 should fix this.

[e14f4152-4982-4ace-8c95-73a0599b109b]

comment:33

I just noticed that Sphinx fails to build the reference manual. The "problem" may be in template.py:

Sphinx error:
'utf8' codec can't decode bytes in position 746-749: invalid data