On 02.07.2008, at 17:49, Jeff Hammel wrote:
> On Tue, Jul 01, 2008 at 10:35:01PM +0200, Christopher Lenz wrote:
>> On the patch itself:
>>
>> * What's the use case for configurable default query_args? Couldn't
>> the default options for the query module itself be reused instead, if
>> possible? Or just some sane default like all open tickets?
>
> Now I'm using [query]\ndefault_anonymous_query.  Is that better?  I  
> don't want to use default_query as one of these fields is (by  
> default) owner.

Good point. default_anonymous_query should work for now, we can also  
switch to a separate config option, or even a sensible hard-coded  
default, later if needed.

>> * The excluded_fields option refers to fields not included with Trac
>> core. This should probably default to an empty list instead, if it is
>> even necessary at all (note how I like to avoid adding config
>> options :P )
>
> I've left this in for now.  This is a solution to 
> http://trac-hacks.org/ticket/3178 
>  which is necessary since the timingandestimation plugin uses JS to  
> markup the fields.  A couple of alternatives here:
>
> * take out the excluded_fields entirely;  this will make the patch  
> cleaner, but will break timingandestimation.  If the patch is  
> committed to core, then I'll ticket that plugin noting the issue and  
> maybe try to make a patch.
>
> * instead of having an option, just have these hard-coded fields  
> with a "# XXX" noting the reason that this is here.  Again, its  
> probably worth alerting timingandestimation about this so we can  
> kill this entirely.

Yeah, this really should be fixed in TimingAndEstimation. Providing an  
excluded_fields option definitely makes sense, but the name needs  
improvement as it sounds way too broad… maybe something like  
"unlinked_fields" or such?

>> * The query_link thing is kind of brute force. Would be nicer to just
>> use the **kwargs of req.href to construct the URL qith query string
>> params. And it probably should be `query_link(owner=ticket.owner)`
>> instead of `query_link('owner', ticket.owner)`. It would probably be
>> possible to have query_link be just a partial binding of req.href.
>
> I've kept this as a function in web_ui.py, though it does feel a bit  
> brute force.  I haven't changed it to be (**kw) since it will always  
> take one key, value pair.  If its really better to have it use **kw,  
> I can do this.

Makes sense.

> Also, maybe its better just to put this function in the template?   
> Another alternative is just to markup the fields themselves and not  
> bother with the function at all.  Any thoughts on this?

On second though, the query_link stuff could probably be moved into  
TicketModule._prepare_fields() and the template would just access  
field members. That'd also move the excluded_fields logic out of the  
template and into the controller. That feels slightly cleaner to me,  
but no biggie either way.

>> On a related note, I'd prefer if the links in the ticket properties
>> sbox (the yellow box) would be more subtle (I'm assuming they're the
>> default red right now), possibly even only really discoverable on
>> hover. Many pages in Trac have already gotten way to "linky"; at some
>> point you end up with content where every second word is a red link,
>> which is ugly and distracting :P
>
> They are indeed the default red.
> Just to play Devil's advocate, I don't mind super-linky pages and  
> actually like them.  But I'm not against making these links more  
> subtle.  Is there an existing class that I can use to make these  
> more subtle?  I don't mind adding a class, but I don't really have  
> much aptitude for UI, so any CSS I would do probably wouldn't be  
> wonderful.  Still I can try if you like.

Never mind, you can just leave this as is. I (or someone else) might  
get a chance to make "non-essential" links more subtle sometime in the  
future.

Thanks,
--
Christopher Lenz
   cmlenz at gmx.de
   http://www.cmlenz.net/



--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Trac 
Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to