Ticket #2821

All,

I've implemented a solution to http://trac.edge
wall.org/ticket/2821 in 
https://svn.edgewall.com/repos/trac/sandbox/ticket-2821  (This builds on work 
done by cboos earlier.)

I'd like to target merging this to trunk on Wednesday (CDT).
 The UI could 
probably be improved; suggestions (and patches ;) )
welcome.

Eli

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

On 5/11/07, Eli Carter <eli.cartercommprove.com> wrote:
> I've implemented a solution to http://trac.edge
wall.org/ticket/2821 in
> https://svn.edgewall.com/repos/trac/sandbox/ticket-2821  (This builds on work
> done by cboos earlier.)

I quickly looked through the changes on the branch, but it
seems to me
there are a lot of changes unrelated to the #2821 problem.
Shouldn't
those just be on trunk and merged to the branch? And is
there some
sort of design document on what you're going to implement to
fix
#2821?

Regards,

Manuzhai

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

Eli Carter wrote:
> All,
>
> I've implemented a solution to http://trac.edge
wall.org/ticket/2821 in 
> https://svn.edgewall.com/repos/trac/sandbox/ticket-2821  (This builds on work 
> done by cboos earlier.)
>
> I'd like to target merging this to trunk on Wednesday
(CDT).  The UI could 
> probably be improved; suggestions (and patches ;) )
welcome.
>   

Well, I'd like to get a bit more time to review/change this
branch.
I tried it yesterday evening and there's a few things I'd
like to improve:
 - the presentation of the "row" fields could be
enhanced (in particular 
the verbose flag should only be a compatibility flag for 
row=description, not an additional setting)
 - the list of checkboxes for selecting the columns is a bit
too intrusive

OTOH, the status of that branch is OK, so those changes
could as well 
happen on trunk, at your convenience.

-- Christian


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

On Wednesday 16 May 2007, Christian Boos wrote:
> Well, I'd like to get a bit more time to review/change
this branch.
> I tried it yesterday evening and there's a few things
I'd like to improve:
>  - the presentation of the "row" fields could
be enhanced (in particular 
> the verbose flag should only be a compatibility flag
for 
> row=description, not an additional setting)

Well, the verbose flag added reporter, age, and description,
which didn't 
match 1:1 with the row selection.  And without it, there
isn't a way to get 
the age of the ticket.  (For that matter, there isn't a way
to get the ticket 
creation time or the last modified time; both of which would
be useful.)

>  - the list of checkboxes for selecting the columns is
a bit too intrusive

Yeah, it's not real pretty.  For javascript-enabled clients,
making it a 
collapsable box would probably be a small step in the right
direction.  
Otherwise, do you have a suggestion on how that could be
presented better?

> OTOH, the status of that branch is OK, so those changes
could as well 
> happen on trunk, at your convenience.

I'll hold off the merge for today; try to catch me in IRC
and we can address 
some of this.

Thanks,

Eli

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

Eli Carter wrote:
> On Wednesday 16 May 2007, Christian Boos wrote:
>   
>> Well, I'd like to get a bit more time to
review/change this branch.
>> I tried it yesterday evening and there's a few
things I'd like to improve:
>>  - the presentation of the "row" fields
could be enhanced (in particular 
>> the verbose flag should only be a compatibility
flag for 
>> row=description, not an additional setting)
>>     
>
> Well, the verbose flag added reporter, age, and
description, which didn't 
> match 1:1 with the row selection.  And without it,
there isn't a way to get 
> the age of the ticket.  

I think that as soon as we have a "row" field, we
could add the reporter 
and age information.
It makes for fewer choices in the selector panel and looks
more like the 
way it did in 0.10.
Also the row fields are displayed in a cleaner way. See
attached patch.

> (For that matter, there isn't a way to get the ticket 
> creation time or the last modified time; both of which
would be useful.)
>
>   

See the recent #5326 and #4966 tickets.

>>  - the list of checkboxes for selecting the columns
is a bit too intrusive
>>     
>
> Yeah, it's not real pretty.  For javascript-enabled
clients, making it a 
> collapsable box would probably be a small step in the
right direction.  
> Otherwise, do you have a suggestion on how that could
be presented better?
>   

Without javascript, I would prefer to have the list of
fields below the 
table, if possible.
Then with javascript, we move them back on the top, but
collapsed and we 
leave a [Customize...] link at the topleft of the table.

-- Christian

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

On Wednesday 16 May 2007, Christian Boos wrote:
> Eli Carter wrote:
> > On Wednesday 16 May 2007, Christian Boos wrote:
> >   
> >> Well, I'd like to get a bit more time to
review/change this branch.
> >> I tried it yesterday evening and there's a few
things I'd like to 
improve:
> >>  - the presentation of the "row"
fields could be enhanced (in particular 
> >> the verbose flag should only be a
compatibility flag for 
> >> row=description, not an additional setting)
> >>     
> >
> > Well, the verbose flag added reporter, age, and
description, which didn't 
> > match 1:1 with the row selection.  And without it,
there isn't a way to 
get 
> > the age of the ticket.  
> 
> I think that as soon as we have a "row"
field, we could add the reporter 
> and age information.
> It makes for fewer choices in the selector panel and
looks more like the 
> way it did in 0.10.
> Also the row fields are displayed in a cleaner way. See
attached patch.

Patch applied to the branch, thanks. 
That looks much better.  However, the amount of vertical
whitespace is a bit 
much, I think.

> >>  - the list of checkboxes for selecting the
columns is a bit too 
intrusive
> >>     
> >
> > Yeah, it's not real pretty.  For
javascript-enabled clients, making it a 
> > collapsable box would probably be a small step in
the right direction.  
> > Otherwise, do you have a suggestion on how that
could be presented better?
> >   
> 
> Without javascript, I would prefer to have the list of
fields below the 
> table, if possible.

To customize my query, I don't want to have to scroll to the
bottom of the 
page.

> Then with javascript, we move them back on the top, but
collapsed and we 
> leave a [Customize...] link at the topleft of the
table.

Thanks again,

Eli

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

Eli Carter wrote:
> Patch applied to the branch, thanks. 
> That looks much better.  However, the amount of
vertical whitespace is a bit 
> much, I think.
>   

Ok, I've fixed that in r4506. I was somehow surprised about
how my Opera 
browser was behaving while tweaking the stylesheet. I hope
the change 
doesn't disturb too much the other browsers (works fine in
FF, IE7 and Op9).

>   
>>>>  - the list of checkboxes for selecting the
columns is a bit too intrusive
>>>>         
>>> Yeah, it's not real pretty.  For
javascript-enabled clients, making it a 
>>> collapsable box would probably be a small step
in the right direction.  
>>> Otherwise, do you have a suggestion on how that
could be presented better?
>>>   
>>>       
>> Without javascript, I would prefer to have the list
of fields below the 
>> table, if possible.
>>     
>
> To customize my query, I don't want to have to scroll
to the bottom of the 
> page.
>   

Ok, see r4508-4509.
Looks good enough for a merge.

-- Christian


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

On Thursday 17 May 2007, Christian Boos wrote:
> 
> Eli Carter wrote:
> > Patch applied to the branch, thanks. 
> > That looks much better.  However, the amount of
vertical whitespace is a 
bit 
> > much, I think.
> >   
> 
> Ok, I've fixed that in r4506. I was somehow surprised
about how my Opera 
> browser was behaving while tweaking the stylesheet. I
hope the change 
> doesn't disturb too much the other browsers (works fine
in FF, IE7 and Op9).

Ah, much better.  Thanks!  (konqueror here)

> >>>>  - the list of checkboxes for
selecting the columns is a bit too 
intrusive
> >>>>         
> >>> Yeah, it's not real pretty.  For
javascript-enabled clients, making it a 
> >>> collapsable box would probably be a small
step in the right direction.  
> >>> Otherwise, do you have a suggestion on how
that could be presented 
better?
> >>>   
> >>>       
> >> Without javascript, I would prefer to have the
list of fields below the 
> >> table, if possible.
> >>     
> >
> > To customize my query, I don't want to have to
scroll to the bottom of the 
> > page.
> >   
> 
> Ok, see r4508-4509.
> Looks good enough for a merge.

The collapsable boxes look great.  Thanks! 

I'll merge sometime today.

Small nit for the previous patch (in [5405]), you changed
the API for rows 
from rows=None to rows=[]; but didn't make the same change
for cols.  Any 
particular reason for that?

Eli

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

Eli Carter wrote:
> I'll merge sometime today.
>   

Good! Then we can continue with the time fields.

> Small nit for the previous patch (in [5405]), you
changed the API for rows 
> from rows=None to rows=[]; but didn't make the same
change for cols.  Any 
> particular reason for that?

It was interesting for rows, in order to keep the append()
done just 
after simple, but not really needed for cols.
Besides, the "rows" argument was a new addition to
the API so changing 
it wouldn't break anything 
For cols, it's quite likely that there exists code setting
it explicitly 
to None, so changing it now is perhaps not a good idea.

-- Christian



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

On Thursday 17 May 2007, Christian Boos wrote:
> 
> Eli Carter wrote:
> > I'll merge sometime today.
> Good! Then we can continue with the time fields.

Merged.

> > Small nit for the previous patch (in [5405]), you
changed the API for rows 
> > from rows=None to rows=[]; but didn't make the
same change for cols.  Any 
> > particular reason for that?
> 
> It was interesting for rows, in order to keep the
append() done just 
> after simple, but not really needed for cols.
> Besides, the "rows" argument was a new
addition to the API so changing 
> it wouldn't break anything 
> For cols, it's quite likely that there exists code
setting it explicitly 
> to None, so changing it now is perhaps not a good
idea.

Fair enough for now.  At some point, we may want to revisit
that API.

Hmmm.... I'm seeing an odd visual artifact here....

 1. go to custom query
 2. add a filter
 3. update
 4. expand columns box
 5. remove a filter
 6. note the duplication of the columns box header

Hmm... works fine in Firefox, so it looks like this is a
Konqueror bug.  Eh.

Eli

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