Base template cleanup and related CSS/HTML/template fixes

[sinergatis]

I have pushed this branch onto your repository in order to make collaboration easier, in case you have the time to provide further fixes. This ongoing pull request is mainly related to #43 and other template fixes scattered through comments and the issue tracker.

[sinergatis]

• [x] Update the permissions (logged in, can upload, etc) checks to match the current Pathagar settings 8ea9380
• [x] Add a basic footer element for all pages 6f65b7f
• [x] Revise navbar structure so the search form does not cause alignment problems in small devices 316c2c7
• [x] Make the RSS link on the navbar point to the corresponding .atom page 7c8a028

Other issues:

• [x] Revise author_list to make sure the search box points to the right page, and q is handled properly. Also, it seems to be throwing a JS error for datatables.
• [x] Remove book_list.html, as it is currently unused (rename book_list2.html instead)?
• [x] Decide about the RSS inclusion
• [x] Decide what content should be in the footer
• [x] Make the navbar collapsable on mobile, using standard bootstrap features
• [x] Revise CSS for a elements
• [x] Remove underscore JS? Not (yet?).
• [x] Cleanup title block

[sinergatis]

RSS

Yes though I'm not sure if that's the best method. calibre's OPDS feed, as well as possibly others or many, place them all together in some menu. That's might be the ideal way though some OPDS overhaul.

Hmmm, I am not really that familiar with how the reader software usually uses the OPDS feed. Might be worth checking the docs and making some tests, but I believe we might end up always including a "root" RSS feed regardless the page we are in (even if its not linked to the RSS navbar button, and it is included somewhere else such as in the footer). /catalog.atom is probably a safe choice - it basically contains "links" to the other feeds (instead of actual book listings) which hopefully the reader understands and crawls as needed.

[sinergatis]

Footer

With these changes, a sticky footer element is present on all pages (except for login) - I haven't gone into deciding which content should be there.

Maybe you're referring to book count in the footer? I had realized that was broken and hadn't gotten around to fixing it yet. I'm not sure if I prefer to have that or not in the footer. It was originally in a stats box, though I forgot if it was included only in list view. I'm undecided if book count should be included in the footer, though additional links like for tag list, a possible about page, contact, etc, are probably well placed there.

I like the idea of having the footer look like a more traditional footer (ie. copyright of sorts, contact info, version info, etc), and I'm not really opposed to having the book count there - but including the book count would imply making sure all the templates receive the context parameter (books at the moment), which is doable but needs some thinking (probably by using a context manager, that has some performance implications).

As for the tags page, I'm wondering if we should move it back to the top navbar? The navbar is (still) not too crowded, and it feels like it merits some prominence - I'm not sure.

[sinergatis]

CSS for a elements

Currently it seems that all a elements have a color: inherit rule that makes them the same color as the main text, and for those links that need to be highlighted a class is used (such as a.list_title). This feels a bit unorthodox and it's causing some inconveniences that I'm not sure if they are intended (for example, it is not evident that the link to tags on the footer is really a link). I'm curious about this decision: could you shed some light on it?

Having some links the same color as the text and making them a bit more "discreet" makes sense, but usually the approach is the opposite - keep the bare a elements highlighted, and apply classes to those exceptions instead, and feels a bit more natural UI-wise.

[aristippe]

Awesome! Pushing to my repo is no problem at all and looks to be a good idea since indeed I can more easily look at it. :)

To answer some of your questions:

book_list.html I had left in since I wasn't sure if that was a possible view among a few that in the future, someone could select between. For example, a thumbnail view that shows a grid of book covers, with some toolbar to switch between that and the current list view. The old list view I'm not sure if as a third option is in some ways easier to read (since authors is in a separate column), but it's true, I'm undecided if it's a keeper.

footer. Yeah book count is perhaps not needed in the footer. It could be integrated somewhere in the content area of book_list. Tags I was undecided about if it should be in the footer, but perhaps indeed at the top is a good idea, so it would also show up in the collapsable menu.

RSS: From what I've seen, instead of separate feeds for latest, by author, by title, etc., there's one root view which is sort of like a list of feeds, or a menu, from which one can then select. One example: Authors, Collections, Editors, Tags, Ratings (possible subview to separate views of 1,2,3…), Languages, All, Recent. That seems ideal to have RSS go to one place, a root view that lets you select between different ones. Pathagar's OPDS doesn't work that way at the moment, though I was going to attempt to tackle it someday, though possibly not for a while since of the epub readers I've found, their OPDS support is sort of unsatisfactory, so I was on detour to try to make my own first.

CSS All a elements with color:inherit was my first attempt though I haven't thought through alternatives. Nearly everything that should be a link is a link: author, publisher, tag, and possibly language and other things in the future. Yet the colors as they are now seem ok, with each element styled as they currently are so in context, they show up in some color or not. For instance in book_list, title is blue, authors are black, and tags and publishers are light gray, colored by hierarchy, instead of all links being the same color. Maybe it is better to have a default link color, and individually style links of class book_list_author, book_list_publisher, book_detail_author, … etc.

[aristippe]

Added a commit with the following:

• use sites framework to show site name in templates (OPDS feed contents TODO)
• modify icon style for search box in toolbar
• update some fr translation strings
• remove footer for now since it doesn't show anything

At the moment, I'm not sure what could go into the footer so it's been commented out.

[sinergatis]

Sounds good!

Can you actually tweak the title block a bit, so the base template is the one that has the :: {{ request.site.name }} stuff, basically to make it more consistent (and less prone to forgetting)? Something similar to having this on base.html:

<title>{% block title %}{% endblock %} :: {{ request.site.name }}</title>

and revise the rest of the templates so they just include the page title without the sites stuff?

As for the footer, perhaps we can put a placeholder copyright of sorts, and place a RSS feed icon as well that always points to /catalog.atom - that view does provide the "list of feeds" similar to the link you posted, and would probably allow us to take care of the RSS, at least for the moment?

[sinergatis]

Updated the initial comment with a task about removing underscore.js from base - I'm not sure if this is used anymore?

[aristippe]

The title block change is certainly a good idea. I was happy to find out how to use the sites framework, and then after adding … ":: Sites" to all titles, didn't realize at the time since it's everywhere, there was an easier way.

For a footer, I'm not sure what kind of copyright would be appropriate. I'm excluding the name Pathagar since some might want to deploy with merely the site name, though some custom text specified somewhere, one reason I was thinking of frameworks that extend sites, could be helpful.

Regarding RSS, any thoughts about just including one RSS link at the top that points to catalog.atom? Shows how much I've looked at RSS, I didn't know it was there. :) To me, it seems like the way to go, it's usually the base view in other OPDS providers I've seen, and if one wanted something else, it'd be placed somewhere else like in the footer or another page, much like news sites can have multiple feeds but have a main one. One usually adds a single OPDS feed to an ePub reader, and then after it's set, that's the primary view one uses to retrieve items, however one's reader displays it. I'm not sure if any one person is going to add multiple feeds of different views to their own client, but some might prefer one feed over the other, though I think that'd be rare. I'll wait til I hear from ya before the next commit, but I think that way is simpler, and I'd argue to even not include the other categorized RSS links since I don't think they'd be used much, and I like the cleaner look. :)

As for the line limits in HTML, I agree. I've been fighting perhaps a bug in Pycharm that wraps text despite settings per file type.

For removing base_site.html, I could go either way. I don't plan on changing it though I think it's nicer to keep since it permits other things like styles or more customization if one wanted to add a toolbar or something else. I have no current plans for any of that, so leaving it there, who knows.

underscore.js: Along with jQuery, I have the habit of using both whenever I do JS, though here there hasn't been much yet except for the table style and highlight on mouse hover. For future things, such as if we add the checkbox to do batch ops on book_list, it might come in handy. I forgot the underscore fork name that's also popular and perhaps faster, though I went back to underscore for userscripts since the other seemed to conflict when that and underscore is used on the site. Despite how some more experienced (that's not me) JS programmers prefer pure javascript (for reasons including perf), I like it and it's quite readable. Whenever any decent amount of JS someday gets added, I'll likely use it and my addition was merely habit and prep.

[sinergatis]

Pushed a commit dc6d131 that:

• removes the old book_list.html, as it was unused. Your idea about reintroducing it later sounds good - and one of the benefits of using git is that basically we can check out the older file when that comes is needed! Usually I do prefer that approach (ie. just remove the unused file/function/etc as it will always be in the repository history) in order to avoid going through the trouble of figuring out later in the future if stuff is unused.
• Moved the accounts templates to the main /templates folder, for consistency. It is arguably not the recommended practice (ie. technically it would be more standard to store the templates for books under book/templates, but since we are doing it the other way around it kind of made sense.
• Fixed a small issue with the tag template that was caught when running the tests (one side effect of having the authorization tests in place is that running python manage.py test books.tests.test_permissions.PermissionsTestCase.test_permissions_with_pathagar_settings will crash if an URL fails with a 500 error)-

[sinergatis]

For a footer, I'm not sure what kind of copyright would be appropriate. I'm excluding the name Pathagar since some might want to deploy with merely the site name, though some custom text specified somewhere, one reason I was thinking of frameworks that extend sites, could be helpful.

Well I was thinking something generic ("copyright" might have been a bad choice of wording on my end!), can be as simple as: SITE.NAME OPDS server - powered by <a href="...">Pathagar</a> mainly for acknowledging the original authors efforts, and keeping the footer for clarity and for having room for improvements later.

Regarding RSS, any thoughts about just including one RSS link at the top that points to catalog.atom?

I feel that proper OPDS reader software will not really care much about the placement, and instead automatically find the RSS feed - as long as it is somewhere on the page. And definitely seems that the catalog.atom looks like the right one (and OPDS clients should be able to understand it and reach the rest of the RSS links from it), so yeah, I see your argument about making it the default everywhere. However, I believe the individual RSS links (by-author.atom, etc) probably make sense for other purposes (ie. a user subscribing to the latest feed using a regular feed reader or a not-so-great OPDS reader because he wants to easily be notified of new books, etc). So what about:

• having the catalog RSS feed linked on the top navbar, but slightly less prominent (maybe just using an RSS icon?)
• if the page has a corresponding RSS feed, place also a link to that feed somewhere where it does not impact the UI much (maybe at the bottom of the content, or on the footer)?

If you feel the suggestion makes sense, or it does with minor adjustments, just go for it! :+1:

underscore.js: Along with jQuery, I have the habit of using both whenever I do JS, though here there hasn't been much yet except for the table style and highlight on mouse hover.

No worries - I was just trying to find out if we were really using underscore, as it seemed that it was mostly used on the original codebase and made obsolete by recent changes. I'd personally prefer if we could stick to just one framework (jQuery probably being the logical candidate due to the use of tables, boostrap, etc), unless we are doing some really complex and crazy JS stuff that depends on underscore's specific features, but we can come back to it later if needed! For the purposes of this pull request, keeping it is more than fine by me!

[aristippe]

SITE.NAME OPDS server - powered by Pathagar mainly for acknowledging the original authors efforts, and keeping the footer for clarity and for having room for improvements later.

Ah yes, such a thing is indeed a good idea, though I very much prefer to have it placed somewhere configurable, like in an extended site model so as with site.name, the acknowledgement string can be set without changing the template. At least for my use, I prefer to not have any identifying info, much like Nginx, Django, etc. are not shown.

I believe the individual RSS links (by-author.atom, etc) probably make sense for other purposes (ie. a user subscribing to the latest feed using a regular feed reader or a not-so-great OPDS reader because he wants to easily be notified of new books, etc).

I had forgotten about that. :) Indeed latest and perhaps others are quite useful like for an RSS feed reader, though as with OPDS, HTTP basic auth instead of Django auth will be needed. No doubt, who ever is first in getting around to using the feature will take care of. I will certainly, though it'll be a while before I have a decent OPDS client. As for OPDS clients using one or another, it will be much like RSS: one either pastes a feed link, or when clicking it, there's a default app to handle opds://. So usually, the default catalog.atom will work well for OPDS itself. For instance, here's now Feedbooks looks in Marvin, showing popular (not a bad idea which I might add someday), latest, fiction, and non-fiction:

For the footer style, I had trouble getting the UI element to be vertically centered in a container, but I'll continue to experiment. For the a style using inherit, I agree there might be issues like perhaps default link style in book.summary. I still had trouble getting it to be some default value while styling other others by class and I'll continue trying as well.

[sinergatis]

Ok! Revised the feeds so:

• the top navbar always has a RSS icon that points to catalog (and the RSS button was removed)
• book_list has a RSS icon that points to the specific RSS feed, next to the header (a generic "Books" header has been added)

The icons are generated using fontawesome, basically for making scaling nicer. Feel free to move things around - the idea was basically to keep both the catalog and the specific RSS feeds available to users interested on those, while making them not really intrusive for users who just intend to use the web site.

Also, reintroduced the footer with the generic text.

[sinergatis]

And went ahead and cleaned up the title block as well, plus adding some icons to the navbar buttons (might be due to a severe case of Friday happiness - if you don't like them, just remove them!).

I also tweaked a bit the book list in order to highlight a bit the current ordering (ie. the column name is shown in italic), to provide some help to the user. This should probably be implemented using the datatables-module features (instead of some if list_by ... tags), but the code seems to be a bit convoluted at the moment - perhaps a round of cleaning up that template as well should be needed.

[sinergatis]

... and now the navbar should be "collapsable", taking up less space on devices with small width (or if the browser window is too small, etc).

This hopefully makes the pull request close to being completed. The only parts missing are:

• Revise authors_list.html, as the JS console shows an error related to datatables. Also, the block script stuff should be rewritten (pretty much copy the one at book_list, changing the string)
• CSS cleanup, in particular the a issue mentioned and discussed, but potentially check for unused CSS, etc. I'd also love it if the templates did not contain elements with style=... applied directly.
• a final internalization round to sync the .po and catch any missing strings.

It's probably more efficient if you take care of these, as you are more familiar with datatables and with the i8n stuff. We can probably move the CSS cleanup to a separate issue and handle it later, as there might be more cleaning to do on other templates! The author_list stuff is also not that urgent and can be moved to that issue if you can't find the time or gets too messy.

Short version: feel free to review the changes, move things around or comment back if not happy, and if you have time to check the i8n that would be great!

[aristippe]

Got around to take a look and the new icons look nice! The only thing I might prefer is the RSS links being in the footer. Since a regular user is likely not to use them often, but perhaps only initially add them to an ePub or RSS reader, having them in the footer would make the header cleaner. Small thing that we can change anytime. Was deciding on a style for that, plus if the footer could have a different style, like either no background or maybe some box that isn't flush with the margins. Still thinking about it.

Agree the style="…" inside templates isn't ideal and those are from times I was testing out. Fixed most of them except for the pagination div. Forcing text-align: right !important in .pagination didn't work for some reason. Will look at it sometime. Also fixed the a links by adding classes to all of them. The javascript console error, from what I could find out, is default in dataTables, and other 3rd party style sheets, as an IE hack. Didn't show up in Chrome so I didn't see it but appears in Safari, and likely you're using Firefox. We'll possibly move away from dataTables sometime.

Anything else, I'm ok with merging and taking care of later. Am quite involved with my other project at the moment. :)

[sinergatis]

Cool, thanks! Seems will also be a hectic week for me, but I hope I'll be able to check the new changes in detail and hopefully finally merge later!

No worries about the "style=..." inside tags, I know they are quite handy for quick development and testing! It was more an idea for a future CSS revision round rather than an urgent issue, but thanks for taking a look at them and for the authors page stuff! Weirdly enough the problem shows up on my Chrome as well, but I'll move it to a separate issue (with probably low priority if we are moving away from datatables indeed).

Thanks :+1:

[sinergatis]

Ok, I have been able to check the changes, and they look fine ... so merging! Thanks for your cooperation on this cleanup!

It will however spawn some other low-priority issues - mainly a more in depth CSS cleanup (in particular, currently nearly all the links on the book list seem to have the same style when hovering and when not-hovering?), and the datatables stuff. But the main goal (ie. having a sane base template) has been achieved! :green_heart: