Operations are now worded too verbosely on /admin/content/book page:

I changed those to be edit, permissions, outline (more in style with other drupal modules)

CommentFileSizeAuthor
#9 better_ops.4.patch2.17 KBbeginner
#6 better_ops.3.patch1.16 KBbeginner
#3 better_ops.2.patch1.15 KBbeginner
#1 outline-better_ops.patch991 bytesiva2k

Comments

iva2k’s picture

iva2k commented
Status: Active » Needs review
StatusFileSize
new outline-better_ops.patch991 bytes

Patch attached.

iva2k’s picture

iva2k commented

The original wording in 6.x-0.x-dev was:

edit permissions :: edit order and titles

beginner’s picture

beginner commented
StatusFileSize
new better_ops.2.patch1.15 KB

Nice patch, thanks. I like it.

A couple of comments:

First, check http://drupal.org/patch/create to see the customary way to create a patch (diff -up from a cvs checkout from within the module's directory for crontrib). Still, I had no problem applying your patch. :)

Second, the only thing I don't really like is that the operations are all squeezed on the right end of the window. On a large monitor, it creates a huge gap between the book titles and the operations. This is also due to the default way tables are rendered within Drupal (100% width, even if not all is necessary).
I attach a patch where the only change is that I reduced the 'Book' 'width' from 100% to 50%. The gap is less obvious but it's not ideal either. Is there a better way?

What do you think?

iva2k’s picture

iva2k commented

I see your point with 100%. I made width=100% as a quick hack to make the ops stick together vs spread out. I guess in an ideal world it should be up to the theme how to render the table, and then width= does not belong there. Unfortunately, Drupal is not really separating content from presentation. 50% may work though, unless in some theme it will keep very long titles squished in favor of leaving ops spaced loose. Maybe somebody from theming gurus can voice an opinion on how these things should be done. I did not find any examples in existing modules.

Patch guidelines - I know they are there... it was just too late at night and I wanted to close that issue on my end and move on. I'll put a bookmark for future.

beginner’s picture

beginner commented

Is spread out so bad?
Why not leave the width out altogether? It's not ideal but acceptable and flexible.

The other solution would be to inverse the order by putting the 'operation' before the 'book' title. Actually, it would be much better this way, since the length of the operations is fixed, and the book titles can expand to the right as necessary.

beginner’s picture

beginner commented
StatusFileSize
new better_ops.3.patch1.16 KB

I think I prefer this way (see patch).
What do you think?

iva2k’s picture

iva2k commented

You just read my mind - I was thinking exactly the same. On one custom website I helped with, we had all ops to the left of items (similar to PhpMyAdmin). It makes a lot of sense. I did't like width= solution anyway.

However, there are two caveats: 1. There is no other form in Drupal that I know that does that, so it is inconsistent; 2. The text of operations visually sticks to the book title, making it hard to read (perhaps not on all themes, but it is for sure on acquia_marina that I use)

For these two issues I don't like this left placement of ops. Placement on the right without width=N% sounds more consistent and universal. I'd leave it at that. Spreading of ops is not that big a deal.

iva2k’s picture

iva2k commented

One more thing I forgot to mention - leftside placement will be awesome if operations were icons, not text (again, like PhpMyAdmin). But it should be done via theme in my opinion - it has to look consistent throughout the website.

beginner’s picture

beginner commented
StatusFileSize
new better_ops.4.patch2.17 KB

re: the two caveats:

1) I am all for following Drupal's coding and UI standards when they make sense. But in this case, we'd loose by being consistent. The operations have a fixed length and it really makes sense to stick them to the left. Then the book titles have all the space at the right so that both short and long titles would fit nicely. In other words, I am happy to sacrifice a little consistency.

2) I also noticed how the title stuck to the operations. I have solved that by adding an 'id' and a 'class' so that we can add some padding via css. A padding of 2 or 3 em should fit all themes.

I have also corrected some minor coding standard errors that remained from your initial patch.

Of all the options, I think I like it best this way (see patch).

beginner’s picture

beginner commented
Status: Needs review » Fixed

Thanks iva2k for your help and feedback on this issue.
I committed the last patch.
http://drupal.org/cvs?commit=192666

iva2k’s picture

iva2k commented

I like the final patch. Works for me. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.