CVS edit link for critical_patch
I would like to be maintainer of a new module I have created called flat_book. The flat_book module allows you to collapse onto a single page subtrees of a book below a certain menu depth (this depth may be chosen on the admin page). At the top of a flatten page, a table of contents is provided to allow the user to quickly jump to a particular node on the page. Back to top links are also provided below each node. If the user requests to view a node below the maximum menu depth, he will be redirected to the portion of the flattened page that contains the requested node.This module is useful for organizing large amounts of hierarchical data in a user friendly way. Children of a node in a book are usually related to (talk about the same subject as) the parent. However, if the book tree is deep enough it can become a hassle for the user to drill down all the way to the particular node he is interested in. Furthermore, this flat structure can allow the user to read through entire sections of the book without frequently changing pages.
There is no module that I could find that allows you to flatten particular subtrees of a book. The directory module provides a heirarchical list of node titles, but does not show the bodies of the nodes. You must also have a vocabulary to identify which nodes to place into a directory (ie it does not integrate directly with books). The booktree module shows the titles of a single book in tree form. However, it only works for a single book and the only displays the titles of each page and not the bodies (It also appears to be poorly maintained and portions of the source code are in italian). Finally, the tableofcontents module does provide a table of contents similar to the one my module provides, but the user would have to specify each node where the toc must appear by hand, and would be forced to place section numbers on each node in a way that the toc module would recognize.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | flat_book.zip | 6.84 KB | criticalpatch |
| #10 | flat_book_features.png | 362.15 KB | criticalpatch |
| #6 | flat_book.zip | 6.18 KB | criticalpatch |
| #4 | flat_book.zip | 6.04 KB | criticalpatch |
| #1 | flat_book.zip | 6.04 KB | criticalpatch |
Comments
Comment #1
criticalpatch commentedHere is a zip of the module code.
Comment #2
tjwallace commentedLine 8 of flat_book.install seems unnecessary as `grep -R 'ew_flat_book' .` returns only that line.
Comment #4
criticalpatch commentedSorry this was an oversight. I meant remove the "ew_" prefix. Here is the fixed version.
Comment #5
avpadernoHello, and thanks for applying for a CVS account. I am adding the review tags, and some volunteers will review your code, pointing out what needs to be changed.
Comment #6
criticalpatch commentedThanks for the update:
I've fixed the following issues:
-Overaggressive cache clearing when the admin form is submitted
-Cache now updates properly when a node is updated, added, or deleted
-Redirection prevents node edits
-Typo in function call that sometimes causes crash in book navigation block
The new version is attached.
Comment #7
criticalpatch commentedHello!
Has there been any progress on my module review?
Thanks.
Comment #8
criticalpatch commentedIn order to facilitate easier review, here is some additional documentation.
INSTALLATION:
Install the module as you would any drupal module.
1. Download the zip
2. Extract into your modules folder
3. Enable the module on the admin page, or use "drush enable flat_book"
CONFIGURATION:
The module should immediately start working out of the box. All books will be flattened below the default menu depth (3). The default menu depth can be changed on the admin page at admin/settings/flat_book. In order to properly review this, you will ideally need to create a book and add some pages to it. There is also a flattened book navigation block that can be enabled on the blocks page.
That should be all that's needed to get the module up and running.
As a glimpse into the future, here are some planned features:
Greater admin control:
-Choose which books are to be flattened
-Flatten particular subtrees above or below the default depth (in case we have some particularly complex data in some subtree)
-Allow editing pages in the book subtree
Comment #9
dergachev commented@critical_path,
Perhaps it would be helpful to do the following things to get your application noticed:
1. Create a succinct explanation of the module. A diagram and screenshots (before/after) might be appropriate.
2. Put up a demo site.
3. Get your code on github, so that people can review it without having to download the module.
4. Email maintainers of related modules and get them to pitch in.
@kiamlaluno and all the hard-working Drupal.org volunteers,
This module resulted from Evolving Web's work for McGill University. They had a set of documents in a deep menu tree whose structure was optimized for print publication. When these documents were imported into the new Drupal site, it became apparent that most of the pages were just a few sentences long, and really served to contain other pages. This module allowed us to automatically flatten the tree to 3 levels, which proved to be quite effective.
We received positive feedback about this, and request for additional functionality to allow the administrator to mark a certain node as "Terminal", meaning all its child nodes should be included within it. This is straightforward, but because it wouldn't be paid work, we are waiting on acceptance on drupal.org before we do further work on the module.
Thank you.
Alex Dergachev
Co-founder, Evolving Web
http://evolvingweb.ca
Comment #10
criticalpatch commentedHi,
The module is now on github. It can be checked out via git@github.com:jonathanpullano/flat_book.git
I have changed some comments for clarity, though there have not been any major revisions since I lasted posted the module. I appreciate that making changes to the module makes it more difficult to review, so I have tried to avoid doing any major development. However, I think some of the features I mentioned previously would greatly improve the usability of the module, and so I would appreciate the effort you put into a review.
The module is used on the site http://www.mcgill.ca/study. The books being flattened are in each faculty, available in the faculties and schools section.
An example of a flattened page is available here: http://www.mcgill.ca/study/2010-2011/faculties/music/undergraduate/ug_mu....
Note that this link may break in the future as McGill updates their course calendar, but should be okay for a while. Without the flat_book module enabled, each of the nodes on this page would instead be a child of this page in the menu block on the left, requiring extra clicks from the user to drilldown the menu to the page he desired. The module also provides the Jump To: list of nodes visible in the top right.
The screenshot attached provides a visual overview of the features of the module. A glance at it should be enough to understand the modules intended functionality.
I hope this clarifies any confusion you may have about my module, and I look forward to your review.
Comment #11
xmacinfoIs the file in comment #6 the latest version?
Comment #12
stborchertHey.
There are some small issues (most of them code style; but this can be done after the module has been published and shouldn't prevent the admins from creating your cvs account) and one note:
You could/should use theme_item_list() and l() in
flat_book_menu_tree_tocto create the list.The same for
$prefixin function_flat_book_visit.Apart from that: great module.
Comment #13
xmacinfoI have not reviewed this module completely and looked at the GitHub code only.
The info file should list dependencies:
+
dependencies[] = bookIndentations missing for the HTML part of this file:
flat-book-node-export-html.tpl.php
Marking as needs work.
Please upload the latest version here and I will try to review it later, unless someone RTBC it before. :-)
Comment #14
bonobo commentedThis module does not duplicate any existing code, and this application has been open for several months.
Let's grant the cvs account (if it hasn't been done already) and let any further feedback occur in the issue queue.
It's also worth noting that a contributor started out using the normal channels, and after not getting a cvs account on d.o for two months, used github.
This is exactly the type of fragmentation we do not want happening.
So: +1 to granting cvs access
Any additional feedback can occur in the issue queue for the module.
Comment #15
bonobo commentedChanging title to something more accurate.
Comment #16
btopro commented+1 as well this looks like a great project for those using books and makes the core-optional project that much more useful for educators / others wanting to render content w.o. drastic theme wiring.
Comment #17
criticalpatch commented@xmacinfo I am attaching the latest version to this comment
@stBorchert Thanks the review! I followed both of your suggestions for flat_book_menu_tree_toc, and I cleaned up some of the stylistic issues by running everything through the coder module. I'm sorry I don't understand what you mean about $prefix, I don't see a variable by that name in my code.
In addition to that, I added a few comments, and fixed couple of small bugs in caching and block display, and added a README.
Here is the latest release.
Comment #18
criticalpatch commented@xmacinfo: It had been a while since I refreshed the page when I made the last update, so I didn't see your latest comments. I made the dependencies and indentation fixes on github.
I think from now on I will only make updates on github, so people are not confused about what code to review. Github will always have the latest changes.
@everyone else: Thanks for your good wishes! :)
Comment #19
smk-ka commented- Setting variables in flat_book_install() is not necessary, as variable_get() already returns the default value.
- Wrong use of t() in flat_book_help(): string concatenation is not supported here.
Otherwise this is good to go.
Comment #20
xmacinfoI am reviewing now. I already found one important thing to fix.
I'll be back in a few minutes.
Comment #21
xmacinfoOK. Finally tested.
This is an impressive module that is nicely done.
Here are my comments, where only one is important:
flat_book.module file:
Please add an
// $Id$to mark revision.Hopefully that won’t be necessary once migration to GIT is completed.
README.txt file:
With drush you can typeAdd a column at the end, just before the command: drush en flat_book
Remove the last phrase (end of document):
This module is licensed under GPL.It’s not necessary since the GPL2 LICENCE.txt file will be automatically packaged on d.o.
I believe once these small fixes are done, we will be ready to mark this module RTBC.
Merci.
Comment #22
criticalpatch commented@xmacinfo and smk-ka: Thanks! I've made the changes you suggested.
Comment #23
xmacinfoGreat. RTBC then.
Bienvenue.
Comment #24
avpadernoThank you for your contribution! I am going to update your account.
These are some recommended readings to help with excellent maintainership:
You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
I thank all the dedicated reviewers as well.
Comment #27
avpaderno