After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 619202 - Add the possibility of enabling or disabling specific books
Add the possibility of enabling or disabling specific books
Status: RESOLVED FIXED
Product: devhelp
Classification: Applications
Component: General
git master
Other Linux
: Normal enhancement
: ---
Assigned To: Aleksander Morgado
devhelp-maint
Depends on:
Blocks:
 
 
Reported: 2010-05-20 15:12 UTC by Aleksander Morgado
Modified: 2010-07-12 09:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Single-commit patch (111.57 KB, patch)
2010-06-10 13:47 UTC, Aleksander Morgado
reviewed Details | Review

Description Aleksander Morgado 2010-05-20 15:12:23 UTC
Instead of loading all the available books in the system, Devhelp could have a list of "disabled" books in configuration, modifiable via Devhelp, so that disabled books are skipped from the contents tree and the search results.
Comment 1 Aleksander Morgado 2010-05-20 16:45:01 UTC
Created a new gitorious repo for this.

Changes can be viewed in:
http://gitorious.org/devhelp/devhelp/commits/gb619202-book-manager

Repo can be cloned:
git@gitorious.org:devhelp/devhelp.git

Specific branch is "gb619202-book-manager".

I made some changes in libdevhelp's API, so probably not backwards compatible with previous versions...
Comment 2 Frederic Peters 2010-06-10 12:07:53 UTC
Could you squash your commits, and attach a single patch to this bug report? (you could use git-bz, it's nice).
Comment 3 Aleksander Morgado 2010-06-10 13:40:09 UTC
(In reply to comment #2)
> Could you squash your commits, and attach a single patch to this bug report?
> (you could use git-bz, it's nice).

Hummm... ok, but believe me it will me much easier to review just using meld.
Comment 4 Aleksander Morgado 2010-06-10 13:47:58 UTC
Created attachment 163292 [details] [review]
Single-commit patch

Just saw that devhelp.builder was quite changed outside the new dialog I added, probably because of glade saving it. Anyway, I expect more re-work in the patch, so feel free to comment it.
Comment 5 Frederic Peters 2010-07-02 20:06:10 UTC
Comment on attachment 163292 [details] [review]
Single-commit patch

Thanks Aleksander, and sorry I am slow reviewing it. I finally got time to play with it and here are my few comments:

* books are no longer sorted (there were sorted in the Book tab, and they should be in the new "Book Manager" dialog)
* what about moving the "Book Manager"  to be a "Bookshelf" tab in the Preferences dialog?
* what about instant-applying the activation/deactivation of books?
* there is some margin errors in the "Book Manager" dialog (most important is to get the right edge of the treeview and the button aligned)
* the "Enabled" column checkboxes should be centered (well, unless it's too difficult doing it in GTK+, I didn't check)

Functionality wise it works really well, thanks.
Comment 6 Aleksander Morgado 2010-07-06 07:50:36 UTC
(In reply to comment #5)
> (From update of attachment 163292 [details] [review])
> Thanks Aleksander, and sorry I am slow reviewing it. I finally got time to play
> with it and here are my few comments:
> 

Np, thanks for the review!

> * books are no longer sorted (there were sorted in the Book tab, and they
> should be in the new "Book Manager" dialog)

Oh, will check that.

> * what about moving the "Book Manager"  to be a "Bookshelf" tab in the
> Preferences dialog?

That's actually a good idea. Will create 2 tabs then, one for "Fonts" and the other one for the "Book shelf"

> * what about instant-applying the activation/deactivation of books?

I thought of this in the beginning, but I'm worried it may create a lot of overhead when selecting/deselecting lots of books. Will look at it anyway.

> * there is some margin errors in the "Book Manager" dialog (most important is
> to get the right edge of the treeview and the button aligned)

Oh, will try to fix that when including it in the Preferences dialog.

> * the "Enabled" column checkboxes should be centered (well, unless it's too
> difficult doing it in GTK+, I didn't check)

Not quite sure how to do that also, but will check it.

Cheers.
Comment 7 Aleksander Morgado 2010-07-06 15:47:40 UTC
Hi Frederic, updated the branch in gitorious with the following changes...

> 
> > * books are no longer sorted (there were sorted in the Book tab, and they
> > should be in the new "Book Manager" dialog)
> 
> Oh, will check that.

Done

> 
> > * what about moving the "Book Manager"  to be a "Bookshelf" tab in the
> > Preferences dialog?
> 
> That's actually a good idea. Will create 2 tabs then, one for "Fonts" and the
> other one for the "Book shelf"
> 

Done.

> > * what about instant-applying the activation/deactivation of books?
> 
> I thought of this in the beginning, but I'm worried it may create a lot of
> overhead when selecting/deselecting lots of books. Will look at it anyway.
> 

Done

> > * there is some margin errors in the "Book Manager" dialog (most important is
> > to get the right edge of the treeview and the button aligned)
> 
> Oh, will try to fix that when including it in the Preferences dialog.

Done

> 
> > * the "Enabled" column checkboxes should be centered (well, unless it's too
> > difficult doing it in GTK+, I didn't check)
> 
> Not quite sure how to do that also, but will check it.
> 

As for this one, I tried but no luck. The GtkCellRendererToggle should have xalign and yalign equal to 0.5 (center) by default when not specified. In fact, it seems that when not using GtkBuilder and creating the GtkCellRendererToggle, the checkbox gets centered; but if using GtkBuilder this doesn't happen. Actually, if I change the xalign property with glade for the GtkCellRendererToggle in the GtkBuilder file, whatever value I set it doesn't consider it and leaves it always left-aligned. Not sure if that's my fault or a GtkBuilder issue.
Comment 8 Aleksander Morgado 2010-07-06 16:51:59 UTC
> > 
> > > * the "Enabled" column checkboxes should be centered (well, unless it's too
> > > difficult doing it in GTK+, I didn't check)
> > 
> > Not quite sure how to do that also, but will check it.
> > 
> 
> As for this one, I tried but no luck. The GtkCellRendererToggle should have
> xalign and yalign equal to 0.5 (center) by default when not specified. In fact,
> it seems that when not using GtkBuilder and creating the GtkCellRendererToggle,
> the checkbox gets centered; but if using GtkBuilder this doesn't happen.
> Actually, if I change the xalign property with glade for the
> GtkCellRendererToggle in the GtkBuilder file, whatever value I set it doesn't
> consider it and leaves it always left-aligned. Not sure if that's my fault or a
> GtkBuilder issue.

Fixed this as well in the gitorious branch.
Comment 9 Frederic Peters 2010-07-12 09:22:56 UTC
Thanks a lot; I merged your branch and pushed to git.gnome.org.
Comment 10 Aleksander Morgado 2010-07-12 09:27:25 UTC
Great!