Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#7269 closed enhancement (fixed)

SageNB -- Change table layouts to CSS layouts

Reported by: timdumol Owned by: boothby
Priority: major Milestone: sage-4.3.1
Component: notebook Keywords: sagenb notebook
Cc: was, mpatel Merged in: sagenb-0.5
Authors: Tim Dumol Reviewers: Mitesh Patel
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Changing the table layouts to CSS layouts will make it easier to make future edits, and will pave the path to user styling of the notebook.

Attachments (17)

trac_7269-table-reduction.patch (91.5 KB) - added by timdumol 10 years ago.
Reduces/replaces table layouts with css layouts. Also cleans up the top bar template to be shared among all the templates.
trac_7269-table-reduction.2.patch (95.5 KB) - added by timdumol 10 years ago.
Fixed the "Report a Problem" and "Toggle" links. Removed user_controls.tmpl. Added changes from #7269. Apply this patch only.
trac_7269-table-reduction.2.2.patch (95.5 KB) - added by timdumol 10 years ago.
Fixed the "Report a Problem" and "Toggle" links. Removed user_controls.tmpl. Added changes from #7249. Apply this patch only.
trac_7269-table-reduction.3.patch (95.6 KB) - added by timdumol 10 years ago.
Fixed "None" value in search box due to Jinja2 migration.
trac_7269-table-reduction.4.patch (95.6 KB) - added by timdumol 10 years ago.
Removed an escape that was no longer needed.
trac_7269-table-reduction.5.patch (95.7 KB) - added by timdumol 10 years ago.
Added parentheses to macro actions (needed for Jinja2 migration)
ws_list.js (8.1 KB) - added by mpatel 10 years ago.
Just the separated minimal ws_list.js, for experiments. This is not a patch.
trac_7269-table-reduction.6.patch (203.8 KB) - added by timdumol 10 years ago.
Fixes all given bugs and removes more <table>s. Adds the SASS code with readme file. Apply this patch alone.
trac_7269-table-reduction.7.patch (212.5 KB) - added by timdumol 10 years ago.
A bit of cleanup to make the Se tests less mercurial.
trac_7269-table-reduction.8.patch (212.4 KB) - added by timdumol 10 years ago.
Fixed ws_list.js to include the fix from #5100
trac_7269-table-reduction.9.patch (220.6 KB) - added by mpatel 10 years ago.
Various fixes. Rebased vs. #7650.
trac_7269-table-reduction.10.patch (219.8 KB) - added by timdumol 10 years ago.
Aligns the username, removes the Jinja2 migration, deletes some obsolete files.
trac_7269-table-reduction.11.patch (221.5 KB) - added by mpatel 10 years ago.
Top bar tweaks and doctest fixes. Replaces previous.
trac_7269-table-reduction.12.patch (222.1 KB) - added by mpatel 10 years ago.
Input cell tweaks. Include jQuery just once. Replaces previous.
trac_7269-table-reduction.13.patch (222.1 KB) - added by mpatel 10 years ago.
Apply #7811 to ws_list.js. Replaces previous.
trac_7269-table-reduction.14.patch (222.1 KB) - added by mpatel 10 years ago.
Rebase vs. #7811 v2. Replaces previous.
trac_7269-table-reduction.15.patch (220.8 KB) - added by timdumol 10 years ago.
rebased vs sagenb-0.4.9

Change History (50)

Changed 10 years ago by timdumol

Reduces/replaces table layouts with css layouts. Also cleans up the top bar template to be shared among all the templates.

comment:1 Changed 10 years ago by timdumol

  • Status changed from new to needs_review

This should do the job.

comment:2 Changed 10 years ago by mpatel

Notes from "functional tests:"

  • "Report a Problem," which now appears on more pages, appears to be broken on some.
  • "Toggle" seems to be broken.
  • Should a "Copy of" a docbrowser worksheet include default.css? Should we strip the copy of the sidebar, header, footer, etc.?

I try to take a closer look at the patch itself tomorrow.

comment:3 follow-up: Changed 10 years ago by mpatel

Odds and ends:

  • Should we mark notebook/user_controls.tmpl for deletion?
  • Should we fold in the key parts of #7249's Jinja2 migration patch?

Out of curiousity:

  • What is backwards for?
  • Has anyone seen a inheritance diagram generator for HTML templates?
  • Could we unify two or more of accounts_settings.css, user_management.css, and registration.css for a common user/admin settings theme?

I'll try to look at main.css tomorrow...

comment:4 in reply to: ↑ 3 Changed 10 years ago by timdumol

  • Status changed from needs_review to needs_work

Replying to mpatel:

Odds and ends:

  • Should we mark notebook/user_controls.tmpl for deletion?

Yes. I forgot to do so.

  • Should we fold in the key parts of #7249's Jinja2 migration patch?

Yes. I'll do that now.

Out of curiousity:

  • What is backwards for?

I haven't an idea. I just copied that from the old code.

  • Has anyone seen a inheritance diagram generator for HTML templates?

No, but that would be simply awesome.

  • Could we unify two or more of accounts_settings.css, user_management.css, and registration.css for a common user/admin settings theme?

I actually planned to do a fair bit of changes to the structure (use inheritance instead of includes, etc.), but I think it would be better to put it in another ticket.

I'll try to look at main.css tomorrow...

I actually just copied the old main.css and put it through css2sass so I could edit it in SASS [1] and Compass [2]. I find it much easier to edit CSS that way -- just ask if you want to see the SASS source files. It's just a pity that SASS is Ruby-based, and the only Python alternative (CleverCSS) lacks many of the features and community support that SASS has.

[1] http://sass-lang.com/

[2] http://wiki.github.com/chriseppstein/compass

comment:5 Changed 10 years ago by timdumol

Apparently, backwards makes links go to the parent ("../foo" instead of "foo").

Changed 10 years ago by timdumol

Fixed the "Report a Problem" and "Toggle" links. Removed user_controls.tmpl. Added changes from #7269. Apply this patch only.

Changed 10 years ago by timdumol

Fixed the "Report a Problem" and "Toggle" links. Removed user_controls.tmpl. Added changes from #7249. Apply this patch only.

comment:6 Changed 10 years ago by timdumol

  • Status changed from needs_work to needs_review

The Toggle link problem was caused by a changed selector. Report a problem was due to lack of the JS libraries on certain pages. Both are now fixed.

Changed 10 years ago by timdumol

Fixed "None" value in search box due to Jinja2 migration.

Changed 10 years ago by timdumol

Removed an escape that was no longer needed.

Changed 10 years ago by timdumol

Added parentheses to macro actions (needed for Jinja2 migration)

comment:7 Changed 10 years ago by timdumol

  • Status changed from needs_review to needs_work
  • Work issues set to Bugs on some pages.

comment:8 follow-up: Changed 10 years ago by mpatel

Also:

  • "Log," "Report a Problem," and "Help" don't work on the main Documentation/Help? page.
  • The current Account settings, Error, Sign in, and Sign up pages don't need to load any libraries, I think.
  • The Sage logo on the Sign up page has a border in Firefox.
  • The Upload worksheet and Upload/Create? data file pages use form.submit() instead of type="submit", but these are probably not important.

Thanks for mentioning SASS and Compass! Should we add the .sass files to the repository, to make it easier update the stylesheets in the future?

Reminder to self: Rebase #4714's "jsmath_init" patch.

comment:9 follow-up: Changed 10 years ago by mpatel

On the JS functions (this is mainly for future reference):

  • Active, Archived, and Deleted Worksheets; Published Worksheets for logged-in users; and Upload Worksheet can work with
    history_window
    help
    bugreport
    search_worksheets_enter_pressed
    search_worksheets
    archive_button
    delete_button
    stop_worksheets_button
    download_worksheets_button
    set_worksheet_list_checks
    make_active_button
    empty_trash
    
  • Published Worksheets for anonymous users needs just
    search_worksheets_enter_pressed
    search_worksheets
    

comment:10 Changed 10 years ago by mpatel

On the JS functions (continued):

  • Edit, Text, Undo, Share, Publish, Upload/Create? Data File appear to need
    server_ping_while_alive
    jmolInitialize
    jmolSetCallback
    history_window
    help
    bugreport
    rename_worksheet
    save_worksheet - empty
    save_worksheet_and_close - empty
    worksheet_discard
    go_option
    upload_worksheet_button
    new_worksheet
    download_worksheet
    print_worksheet
    copy_worksheet
    delete_worksheet
    interrupt
    restart_sage
    evaluate_all
    hide_all
    show_all
    delete_all_output
    slide_mode
    cell_mode
    handle_data_menu
    go_system_select
    pretty_print_check
    edit_worksheet
    slide_next
    slide_last
    slide_first
    slide_prev
    

but I think we can drop Jmol, jsMath, TinyMCE, sage3d, etc., and any cell-related functions.

This might seem like too much, but I think it's useful to optimize the JS (and make it modular) just as you've done and are doing for HTML and CSS. Remote embeds (cf. #6855), in particular, may only need a streamlined library.

comment:11 in reply to: ↑ 8 Changed 10 years ago by timdumol

Replying to mpatel:

Also:

  • "Log," "Report a Problem," and "Help" don't work on the main Documentation/Help? page.
  • The current Account settings, Error, Sign in, and Sign up pages don't need to load any libraries, I think.
  • The Sage logo on the Sign up page has a border in Firefox.
  • The Upload worksheet and Upload/Create? data file pages use form.submit() instead of type="submit", but these are probably not important.

Thanks for mentioning SASS and Compass! Should we add the .sass files to the repository, to make it easier update the stylesheets in the future?

Reminder to self: Rebase #4714's "jsmath_init" patch.

I can do so. I'll also put instructions on using SASS + Compass.

comment:12 Changed 10 years ago by mpatel

Sphinx generates the CSS directives "scraped directly from Pygments" (i.e., pygments.css) when the docs are built. If we use a different HTML theme (via pygments_style in conf.py or in theme.conf, cf. #6673), we'll need to regenerate the CSS. I'm not sure about the best way to do this with SASS.

comment:13 Changed 10 years ago by timdumol

How often/when will the HTML theme be changed? Does main.css have to be regenerated on the user-end, or during development?

If it is to be changed on the user-end, @importing pygments.css on main.css would be the best choice. Otherwise, using css2sass on pygments.css and @importing it on main.sass would be more efficient.

comment:14 Changed 10 years ago by mpatel

It may be best to @import, so we can make it possible to customize the introspection and live doc styles. Sphinx also instantiates a Jinja2 template as default.css.

comment:15 in reply to: ↑ 9 ; follow-up: Changed 10 years ago by mpatel

Replying to mpatel:

On the JS functions (this is mainly for future reference):

  • Active, Archived, and Deleted Worksheets; Published Worksheets for logged-in users; and Upload Worksheet can work with

I've separated out what I think is a minimal ws_list.js for these pages. It depends only on jQuery. It'll also depend on jQuery UI after I replace the alerts. It might be useful to update the server status on these pages for authenticated users, that is, add a worksheet-independent ping. I can add that code.

comment:16 Changed 10 years ago by mpatel

A clarification: I'll definitely make a separate ticket for modularizing the notebook JS.

comment:17 Changed 10 years ago by timdumol

I'm suspending work on this until I finish working on the test suite, now that #7310 is done, albeit not yet accepted.

comment:18 in reply to: ↑ 15 ; follow-ups: Changed 10 years ago by timdumol

Now that #7343 is mostly done, I'll resume work on this.

Replying to mpatel:

Replying to mpatel:

On the JS functions (this is mainly for future reference):

  • Active, Archived, and Deleted Worksheets; Published Worksheets for logged-in users; and Upload Worksheet can work with

I've separated out what I think is a minimal ws_list.js for these pages. It depends only on jQuery. It'll also depend on jQuery UI after I replace the alerts. It might be useful to update the server status on these pages for authenticated users, that is, add a worksheet-independent ping. I can add that code.

That would be excellent. Would you mind posting the code?

Changed 10 years ago by mpatel

Just the separated minimal ws_list.js, for experiments. This is not a patch.

comment:19 in reply to: ↑ 18 Changed 10 years ago by mpatel

Replying to timdumol:

Now that #7343 is mostly done, I'll resume work on this.
[...]
That would be excellent. Would you mind posting the code?

I've attached just the separated, minimal ws_list.js, which appears to be enough for the intended pages (except for the already broken "Download" button).

comment:20 in reply to: ↑ 18 ; follow-up: Changed 10 years ago by mpatel

Replying to timdumol:

Now that #7343 is mostly done, I'll resume work on this.

May I ask what are your plans for this ticket? If you need more from me, just let me know.

comment:21 in reply to: ↑ 20 Changed 10 years ago by timdumol

Replying to mpatel:

Replying to timdumol:

Now that #7343 is mostly done, I'll resume work on this.

May I ask what are your plans for this ticket? If you need more from me, just let me know.

Sorry. I've been a bit caught up with school work, etc. I'll try to resume work on it as soon as my workload lets up.

Changed 10 years ago by timdumol

Fixes all given bugs and removes more <table>s. Adds the SASS code with readme file. Apply this patch alone.

comment:22 Changed 10 years ago by timdumol

  • Report Upstream set to N/A
  • Status changed from needs_work to needs_review

This patch should fix the problems. I have included the SASS source files, with a readme on how to edit them. I hope the patch file size does not daunt anyone -- a large contributor is that both the SASS source files and the generated CSS are included.

Changed 10 years ago by timdumol

A bit of cleanup to make the Se tests less mercurial.

Changed 10 years ago by timdumol

Fixed ws_list.js to include the fix from #5100

Changed 10 years ago by mpatel

Various fixes. Rebased vs. #7650.

comment:23 Changed 10 years ago by mpatel

V9:

  • Rebased for #7650.
  • Fixes several failed doctests.
  • Adds ws_list.js to the "Help" and "Log" pages.
  • Re-centers the "Help" page.

Problems:

  • Live doc worksheets don't render, e.g.,
            exceptions.UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 1294: ordinal not in range(128)
    
  • Minor: The notebook settings page sections are no longer centered(?).
  • Minor: Can we remove html/notebook/doc.html and notebook.html_doc? It appears to be redundant.
  • Minor: The user name is not aligned with "Toggle," "Home," etc.

"Heads up" note: I'm about to

  • Revisit #7650 to try to keep cell.py doctests from taking too much memory.
  • Rebase my patch for #7666, which will visit many .py, .js, and .html files, against this ticket. The local queue:
    sagenb_7483.patch
    sagenb_7482.patch
    sagenb-7514.patch
    trac_7648-missing_indent.patch
    trac_7650-sagenb_doctesting.patch
    trac_7269-table-reduction.patch
    trac_7666-alphanumeric_cell_ids.patch
    

comment:24 Changed 10 years ago by timdumol

Jinja2 disables automatic conversion from byte strings to unicode strings. This causes problems with unicode titles/cells. To fix this cleanly (without too much cruft) will require a clean MVP/MVC (Model-View-Presenter/Controller?) separation. I'm removing the jinja2 conversion from this patch, in the meantime.

To note, ws_list.js is the default js include, and so was already included in the Help and Log pages.

I have not noticed that the settings page was centered. IMHO though, I think flush left is cleaner looking (nice straight left edge).

This patch update deletes the obsolete files, aligns the username and removes the Jinja2 migration.

Changed 10 years ago by timdumol

Aligns the username, removes the Jinja2 migration, deletes some obsolete files.

comment:25 Changed 10 years ago by mpatel

  • Work issues Bugs on some pages. deleted

To note, ws_list.js is the default js include, and so was already included in the Help and Log pages.

Oops. I apologize for this. I'm about to attach V11, which

  • May make the top bar display better in narrow windows or those with large fonts.
  • ttile --> title in top_bar.html.
  • Adds a tr to worksheet_listing.html (WebKit error).
  • Fixes failed doctests in template.py:
    • Restores the previous template.py doctests.
    • Adds '# not tested' to the Jinja2 tests.
  • Fixes a failed doctest in notebook.py.

Changed 10 years ago by mpatel

Top bar tweaks and doctest fixes. Replaces previous.

Changed 10 years ago by mpatel

Input cell tweaks. Include jQuery just once. Replaces previous.

comment:26 Changed 10 years ago by mpatel

  • Milestone set to sage-4.3.1
  • Priority changed from minor to major
  • Reviewers set to Mitesh Patel
  • Status changed from needs_review to positive_review
  • Type changed from defect to enhancement

V12:

  • Removes the jQuery include from a few pages, since it's now included in base.html. Including it twice sometimes caused a "mod-2" problem in Firefox --- the browser would claim that (and act as if) the resource jquery-1.3.2.min.js was missing.
  • Tweaks input cell padding so that text does not move by a pixel on focus or blur (I hope). Text is still very likely to move when a cell is focused for the first time after reloading a page, since the sizing algorithms in cell.py and notebook_lib.js are different.

Someone should review my changes, of course, and perhaps also comment on the slightly changed layouts. I think the button/toolbars wrap differently than before, e.g., when the browser width is narrow and/or the font size is large. Does this cause problems on mobiles or when giving presentations?

Positive review! This is great work! I'm still quite new to SASS and Compass, but it seems that they'll make it much easier to edit and manage SageNB stylesheets.

By the way, can we use SASS, Compass, and/or another tool to find unused CSS directives?

Changed 10 years ago by mpatel

Apply #7811 to ws_list.js. Replaces previous.

comment:27 Changed 10 years ago by mpatel

V13 just applies #7811 to ws_list.js. I believe we still use notebook_lib.js for logged in users, but I've made the change, for the sake of completeness. If it's OK, I'd like to postpone "subtracting" ws_list.js from notebook_lib.js until after published interacts (cf. #6855) are enabled.

Changed 10 years ago by mpatel

Rebase vs. #7811 v2. Replaces previous.

comment:28 Changed 10 years ago by was

I can't merge this into sagenb-0.4.8 (which I'm about to release). Please rebase it once sage-4.3.1.alpha0 comes out with this new sagenb. Thanks!

applying trac_7269-table-reduction.14.patch
patching file sagenb/notebook/notebook.py
Hunk #2 succeeded at 950 with fuzz 2 (offset 3 lines).
Hunk #3 FAILED at 1265
Hunk #4 succeeded at 1334 with fuzz 2 (offset 9 lines).
Hunk #5 succeeded at 1407 with fuzz 1 (offset 9 lines).
Hunk #6 succeeded at 1441 with fuzz 1 (offset 9 lines).
Hunk #7 FAILED at 1467
Hunk #8 succeeded at 1486 with fuzz 1 (offset 9 lines).
Hunk #9 succeeded at 1589 with fuzz 1 (offset 9 lines).
Hunk #10 FAILED at 1608
Hunk #11 succeeded at 1651 with fuzz 1 (offset 9 lines).
Hunk #12 FAILED at 1706
Hunk #13 succeeded at 1748 with fuzz 1 (offset 10 lines).
Hunk #14 FAILED at 1757
5 out of 14 hunks FAILED -- saving rejects to file sagenb/notebook/notebook.py.rej
patching file sagenb/notebook/twist.py
Hunk #7 succeeded at 960 with fuzz 2 (offset 2 lines).
patching file sagenb/notebook/worksheet.py
Hunk #1 FAILED at 1712
Hunk #2 FAILED at 1763
Hunk #3 FAILED at 2300
Hunk #4 succeeded at 2442 with fuzz 1 (offset 87 lines).
Hunk #5 succeeded at 2458 with fuzz 1 (offset 87 lines).
3 out of 6 hunks FAILED -- saving rejects to file sagenb/notebook/worksheet.py.rej
abort: patch failed to apply

Changed 10 years ago by timdumol

rebased vs sagenb-0.4.9

comment:29 Changed 10 years ago by timdumol

Rebased version posted.

comment:30 Changed 10 years ago by mpatel

Can you try applying #7650 first, then V14? That should give just one, ignorable failure and much less "fuzz". I apologize for not being more explicit about this dependency.

comment:31 Changed 10 years ago by mpatel

Of course, this assumes #7650 is reviewed.

comment:32 Changed 10 years ago by was

  • Resolution set to fixed
  • Status changed from positive_review to closed

Merged into sagenb-0.5. I was able to apply trac_7269-table-reduction.15.patch just fine without merging in #7650 first. Yeah. So this is in and pushed to the official repo.

comment:33 Changed 10 years ago by mvngu

  • Merged in set to sagenb-0.5
Note: See TracTickets for help on using tickets.