Ticket #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 | Work issues: | |
| Report Upstream: | N/A | Reviewers: | Mitesh Patel |
| Authors: | Tim Dumol | Merged in: | sagenb-0.5 |
| 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
Change History
Changed 4 years ago by timdumol
-
attachment
trac_7269-table-reduction.patch
added
comment:1 Changed 4 years ago by timdumol
- Status changed from new to needs_review
This should do the job.
comment:2 Changed 4 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: ↓ 4 Changed 4 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 4 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.
comment:5 Changed 4 years ago by timdumol
Apparently, backwards makes links go to the parent ("../foo" instead of "foo").
Changed 4 years ago by timdumol
-
attachment
trac_7269-table-reduction.2.patch
added
Fixed the "Report a Problem" and "Toggle" links. Removed user_controls.tmpl. Added changes from #7269. Apply this patch only.
Changed 4 years ago by timdumol
-
attachment
trac_7269-table-reduction.2.2.patch
added
Fixed the "Report a Problem" and "Toggle" links. Removed user_controls.tmpl. Added changes from #7249. Apply this patch only.
comment:6 Changed 4 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 4 years ago by timdumol
-
attachment
trac_7269-table-reduction.3.patch
added
Fixed "None" value in search box due to Jinja2 migration.
Changed 4 years ago by timdumol
-
attachment
trac_7269-table-reduction.4.patch
added
Removed an escape that was no longer needed.
Changed 4 years ago by timdumol
-
attachment
trac_7269-table-reduction.5.patch
added
Added parentheses to macro actions (needed for Jinja2 migration)
comment:7 Changed 4 years ago by timdumol
- Status changed from needs_review to needs_work
- Work issues set to Bugs on some pages.
comment:8 follow-up: ↓ 11 Changed 4 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: ↓ 15 Changed 4 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 4 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 4 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 4 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 4 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 4 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: ↓ 18 Changed 4 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 4 years ago by mpatel
A clarification: I'll definitely make a separate ticket for modularizing the notebook JS.
comment:17 Changed 4 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: ↓ 19 ↓ 20 Changed 4 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 4 years ago by mpatel
-
attachment
ws_list.js
added
Just the separated minimal ws_list.js, for experiments. This is not a patch.
comment:19 in reply to: ↑ 18 Changed 4 years ago by mpatel
comment:20 in reply to: ↑ 18 ; follow-up: ↓ 21 Changed 4 years ago by mpatel
comment:21 in reply to: ↑ 20 Changed 4 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 3 years ago by timdumol
-
attachment
trac_7269-table-reduction.6.patch
added
Fixes all given bugs and removes more <table>s. Adds the SASS code with readme file. Apply this patch alone.
comment:22 Changed 3 years ago by timdumol
- Status changed from needs_work to needs_review
- Report Upstream set to N/A
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 3 years ago by timdumol
-
attachment
trac_7269-table-reduction.7.patch
added
A bit of cleanup to make the Se tests less mercurial.
Changed 3 years ago by timdumol
-
attachment
trac_7269-table-reduction.8.patch
added
Fixed ws_list.js to include the fix from #5100
Changed 3 years ago by mpatel
-
attachment
trac_7269-table-reduction.9.patch
added
Various fixes. Rebased vs. #7650.
comment:23 Changed 3 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 3 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 3 years ago by timdumol
-
attachment
trac_7269-table-reduction.10.patch
added
Aligns the username, removes the Jinja2 migration, deletes some obsolete files.
comment:25 Changed 3 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 3 years ago by mpatel
-
attachment
trac_7269-table-reduction.11.patch
added
Top bar tweaks and doctest fixes. Replaces previous.
Changed 3 years ago by mpatel
-
attachment
trac_7269-table-reduction.12.patch
added
Input cell tweaks. Include jQuery just once. Replaces previous.
comment:26 Changed 3 years ago by mpatel
- Priority changed from minor to major
- Reviewers set to Mitesh Patel
- Type changed from defect to enhancement
- Status changed from needs_review to positive_review
- Milestone set to sage-4.3.1
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 3 years ago by mpatel
-
attachment
trac_7269-table-reduction.13.patch
added
Apply #7811 to ws_list.js. Replaces previous.
comment:27 Changed 3 years ago by mpatel
Changed 3 years ago by mpatel
-
attachment
trac_7269-table-reduction.14.patch
added
Rebase vs. #7811 v2. Replaces previous.
comment:28 Changed 3 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 3 years ago by timdumol
-
attachment
trac_7269-table-reduction.15.patch
added
rebased vs sagenb-0.4.9
comment:29 Changed 3 years ago by timdumol
Rebased version posted.
comment:30 Changed 3 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 3 years ago by mpatel
Of course, this assumes #7650 is reviewed.
comment:32 Changed 3 years ago by was
- Status changed from positive_review to closed
- Resolution set to fixed
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.

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