6.0.0-git
2019-04-24

[#2265] Add better shared resource support to menus
Summary Add better shared resource support to menus
Queue Horde Base
Queue Version HEAD
Type Enhancement
State Rejected
Priority 1. Low
Owners
Requester csacca (at) thecsl (dot) org
Created 2005-07-13 (5033 days ago)
Due
Updated 2006-04-18 (4754 days ago)
Assigned
Resolved 2006-04-18 (4754 days ago)
Milestone
Patch No

History
2006-04-18 10:05:29 Jan Schneider State ⇒ Rejected
 
2006-03-28 13:54:59 Jan Schneider Comment #19
State ⇒ Feedback
Reply to this comment
Are you going to update the patch?
2005-10-24 14:15:35 Jan Schneider Comment #18 Reply to this comment
One thing left: the patch loosed the current option highlighting in 
the menu's drop down list, i.e. it shows +/- again where we displayed 
the options in different colors instead.
2005-10-24 14:03:54 Jan Schneider Deleted Original Message
 
2005-08-17 17:08:24 csacca (at) thecsl (dot) org Comment #17
New Attachment: kron_and_nag_menu_v5[1].patch Download
Reply to this comment
Erg, I found stupid mistake in my patch.  I forgot to add in 'global 
$browser;' to the functions so they puked.  I've added the line to 
each function and now they work correctly.



-Chris
2005-08-10 12:02:58 Jan Schneider State ⇒ Accepted
Version ⇒ HEAD
 
2005-08-10 12:01:58 Jan Schneider Deleted Original Message
 
2005-08-10 12:01:49 Jan Schneider Deleted Original Message
 
2005-08-10 12:01:41 Jan Schneider Deleted Original Message
 
2005-08-09 15:46:02 csacca (at) thecsl (dot) org Comment #16
New Attachment: kron_and_nag_menu_v5.patch
Reply to this comment
Double-quotes are slower since the intepreter needs to check the
strings to variable interpolation.
Makes sense...
You need to update your patch to
use double-quotes for gettext strings otherwise the strings won't get
added to the list of things to translate.
Done
2005-08-09 02:27:21 Matt Selsky Comment #15 Reply to this comment
6. Don't use double-quoted strings unless necessary for variable
interpolation or newlines, etc.
I did this, but I was just kinda wondering why not use double-qoutes?
Double-quotes are slower since the intepreter needs to check the 
strings to variable interpolation.  You need to update your patch to 
use double-quotes for gettext strings otherwise the strings won't get 
added to the list of things to translate.
2005-08-08 16:25:16 csacca (at) thecsl (dot) org Comment #14
New Attachment: kron_and_nag_menu_v4.patch
Reply to this comment
Ok, finished all that... I think.  : P
5. Please don't combine patches to different applications in one file
unless you do the diff from the Horde directory. Try using cvs diff
-uRn.
-uRn didn't work for me, but I did make a patch that from the horde 
directory should work with patch -p0
6. Don't use double-quoted strings unless necessary for variable
interpolation or newlines, etc.
I did this, but I was just kinda wondering why not use double-qoutes?



Thanks for all the quick feedback.



Chris Sacca
2005-08-07 03:54:52 Chuck Hagenbuch Comment #13 Reply to this comment
Done and done, though I'm still not that sure that it's perfect.
Getting better. :)



1. Please rename the functions to 
getTasklistWidget()/getCalendarWidget(). Personal taste, but a bit 
clearer about what the function is.



2. For xhtml, you should use disabled="disabled", not disabled="true".



3. Please spell out boolean and integer in phpdoc comments.



4. You need to echo the result of the function in the templates you 
use it in; you just call it, which results in nothing being output.



5. Please don't combine patches to different applications in one file 
unless you do the diff from the Horde directory. Try using cvs diff 
-uRn.



6. Don't use double-quoted strings unless necessary for variable 
interpolation or newlines, etc.



7. In several places in Nag, the old code would pre-select the default 
tasklist (possibly in Kronolith as well). Your function doesn't 
provide an option for that; it should be added back in.





Not trying to be picky for no reason, but since you've been improving 
it, I thought I'd give you as much feedback as I could. :)
2005-08-04 18:20:51 csacca (at) thecsl (dot) org Comment #12
New Attachment: kron_and_nag_menu_v3.patch
Reply to this comment
This patch needs to be against HEAD since it's new code; it doesn't
apply cleanly at all. Also, in Kronolith, you've lost the <optgroup>
functionality - please restore that. It should be carried over to
Nag, even.
Done and done, though I'm still not that sure that it's perfect.   
Sorry, I'm really not that familiar with the code base yet.



Thanks for helping me out,



Chris Sacca
2005-08-03 22:10:41 Chuck Hagenbuch Comment #11 Reply to this comment
This patch needs to be against HEAD since it's new code; it doesn't 
apply cleanly at all. Also, in Kronolith, you've lost the <optgroup> 
functionality - please restore that. It should be carried over to Nag, 
even.
2005-08-03 20:54:59 Chuck Hagenbuch Deleted Original Message
 
2005-08-03 20:54:47 Chuck Hagenbuch Deleted Original Message
 
2005-08-03 20:54:38 Chuck Hagenbuch Deleted Original Message
 
2005-08-02 16:10:54 csacca (at) thecsl (dot) org Comment #10
New Attachment: kron_and_nag_menu_v2.patch
Reply to this comment
That doesn't work. You show read-only shares in the import and edit
screens, you allow to select no share at all (by adding the My
Calendar etc. entries), and you didn't use gettext for the strings.
ok, I *think* I've made a patch that covers everything.  I takes care 
of showing different permissions, uses gettext, and uses the disabled 
attr in <select> to disallow selecting the non-share entries.



Thoughts?



Chris Sacca
2005-07-28 13:36:38 csacca (at) thecsl (dot) org Comment #9 Reply to this comment
That doesn't work. You show read-only shares in the import and edit
screens, you allow to select no share at all (by adding the My
Calendar etc. entries), and you didn't use gettext for the strings.
Thanks for the feedback.  I'll take another shot at this and attempt 
to correct my mistakes.
2005-07-28 13:20:47 Jan Schneider Comment #8 Reply to this comment
That doesn't work. You show read-only shares in the import and edit 
screens, you allow to select no share at all (by adding the My 
Calendar etc. entries), and you didn't use gettext for the strings.
2005-07-26 14:56:36 csacca (at) thecsl (dot) org Comment #7
New Attachment: kron_and_nag_menu.patch
Reply to this comment
Ok, that makes a lot of sense.  Where should I put the function?
Somewhere in horde/templates/ ? Or maybe in framwork/Menu/ ?
The most sensible place would be in Share.php, but that breaks
backwards compatibility, so I'd go with one function per application,
in lib/App.php.
It took me a little while, but I put something together.  Might apply 
a little fuzzy, but I works under my installation.



Thanks,



Chris


2005-07-18 15:16:22 Chuck Hagenbuch Comment #6 Reply to this comment
Ok, that makes a lot of sense.  Where should I put the function?
Somewhere in horde/templates/ ? Or maybe in framwork/Menu/ ?
The most sensible place would be in Share.php, but that breaks 
backwards compatibility, so I'd go with one function per application, 
in lib/App.php.
2005-07-18 15:14:53 csacca (at) thecsl (dot) org Comment #5 Reply to this comment
Now that this work is being done in so many places, you should move
it to a function so it's not duplicated everywhere.
Ok, that makes a lot of sense.  Where should I put the function?   
Somewhere in horde/templates/ ? Or maybe in framwork/Menu/ ?
2005-07-18 00:41:10 Chuck Hagenbuch Comment #4
State ⇒ Feedback
Reply to this comment
Now that this work is being done in so many places, you should move it 
to a function so it's not duplicated everywhere.
2005-07-13 15:24:59 csacca (at) thecsl (dot) org Comment #3
New Attachment: cvs_nag_sharing_menu.patch
Reply to this comment
Nag Patch
2005-07-13 15:24:20 csacca (at) thecsl (dot) org Comment #2
New Attachment: cvs_kron_sharing_menu.patch
Reply to this comment
Kronolith Patch
2005-07-13 15:23:32 csacca (at) thecsl (dot) org Comment #1
Type ⇒ Enhancement
State ⇒ New
Priority ⇒ 1. Low
Summary ⇒ Add better shared resource support to menus
Queue ⇒ Horde Base
Reply to this comment
I think it would be good if the behavior of the calendar selection 
menu in menu.inc of kronolith was expanded into the create/edit/import 
calendar selection menus as well.  In addition I would find the 
bracketed owner name useful in nag, and possibly other services that 
use shared resources.



I wrote a couple patches (against HEAD) to deal with this in nag and 
kronolith.  Although they don't really address the issues talked about 
in bug #362, I believe they still might be useful.

Saved Queries