Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 166640

Summary: The selected post filter needs to be better indicated
Product: z_Archived Reporter: amehrega
Component: TPTPAssignee: Igor Alelekov <igor.alelekov>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P1 CC: analexee, ewchan, guru.nagarajan, jkubasta, ruslan.scherbakov
Version: unspecifiedKeywords: plan
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard: closed460
Attachments:
Description Flags
Current view
none
proposed changes
none
Proposed view
none
Proposed fix
none
patch for the org.eclipse.hyades.trace.views
none
patch for the org.eclipse.tptp.trace.jvmti.client none

Description amehrega CLA 2006-12-04 10:28:10 EST
The execution statistics view needs to better indicate the active post filter.  It’s currently indicated on top of the view (see the current-view.png attachment).  This is hardly noticeable by first time users.

I suggest indicating the post filter as part of the page's title and provide a hyperlink to allow the user to modify the filter.  See the attached proposed-view.png attachment.
Comment 1 amehrega CLA 2006-12-04 10:28:45 EST
Created attachment 54974 [details]
Current view
Comment 2 amehrega CLA 2006-12-04 10:29:19 EST
Created attachment 54975 [details]
proposed changes
Comment 3 Guru Nagarajan CLA 2006-12-04 11:43:25 EST
This will be addressed in 4.4
Comment 4 amehrega CLA 2007-03-03 15:34:29 EST
Perhaps it might be a good idea to remove the default post filter.  It will be very confusing to novice users.
Comment 5 Ruslan Scherbakov CLA 2007-04-27 06:17:16 EDT
Created attachment 65186 [details]
Proposed view

This proposal is slightly different than proposed in https://bugs.eclipse.org/bugs/attachment.cgi?id=54975
Comment 6 Ruslan Scherbakov CLA 2007-04-27 06:20:04 EDT
Created attachment 65187 [details]
Proposed fix

If the proposed implementation shown in the attached screenshot https://bugs.eclipse.org/bugs/attachment.cgi?id=65186 is OK then the attached patch can be applied in org.eclipse.hyades.trace.views plugin where I have no access to.
Comment 7 Ruslan Scherbakov CLA 2007-04-27 06:25:57 EDT
Review please proposed implementation and apply patch in CVS.

Note that according to initial propose in https://bugs.eclipse.org/bugs/attachment.cgi?id=54975 the text shold be places on the right of the header title. It requires some more changes in SWT Form and its Header layout and possibly reimplement it because current Form widget privately hides FormLayout imlpementation.
Comment 8 amehrega CLA 2007-04-27 10:31:09 EDT
Thanks Ruslan.
The proposed screenshot looks fine but I have a problem with this patch coming in so late in the game.  I3 is expected to close by Monday and I also want Eugene to take a peak at your proposed fix before it's checked in.

I'll try my best to get this into i3 otherwise it will need to wait for i4 or our next release (if it's not approved in i4).
Comment 9 Eugene Chan CLA 2007-04-27 10:46:39 EDT
(In reply to comment #8)
> Thanks Ruslan.
> The proposed screenshot looks fine but I have a problem with this patch coming
> in so late in the game.  I3 is expected to close by Monday and I also want
> Eugene to take a peak at your proposed fix before it's checked in.
> 
> I'll try my best to get this into i3 otherwise it will need to wait for i4 or
> our next release (if it's not approved in i4).
> 

The new title looks fine to me, however, I think it is more appropriate to have this implemented across all statistics views to be consistent and take out the old "(Filter:Default Filter)" string from the view header. I am pretty sure this is not stopship in i4 and will not be approved. Unless this fix for all views is available in i3, I suggest to move this to post 4.4.
Comment 10 Guru Nagarajan CLA 2007-04-27 12:57:09 EDT
Thanks Ruslan. 
Eugene, Ali - Can one of you drive this froward from here - targetting to 4.4 and/or applying to later versions. 
Comment 11 amehrega CLA 2007-04-27 13:21:33 EDT
I agree with Eugene.  The look and feel of all statistic views need to be consistent.  I will have this targeted to future.
Comment 12 amehrega CLA 2007-04-27 18:11:03 EDT
Ruslan,

I briefly checked out the changes and they look great.  Given that i3 has been extended by one week, we can still have this defect checked in if you make the change suggested by Eugene.  We need to have all statistic views look and feel the same.  Each page of the statistic view needs to have the same header:

<Title of the page>
<If applicable, the post filter link that you have included>

As suggested by Eugene, we also need to remove the grey title area.  The following views will need to be changed:

- Execution statistics view (remove the grey title area)
- Memory statistics view (Add page title, add post filter link, and remove grey title)
- Object allocations view (Add page title, add post filter link, and remove grey title) 
- Coverage statistics view (Add page title, add post filter link, and remove grey title)
- Thread analysis view (Add page title, and remove grey title)

I'm reassigning the defect to you.
Comment 13 Igor Alelekov CLA 2008-03-05 11:03:03 EST
Eugene, 
do you think, that filter should be indicated on every page?
E.g. Thread Viewer has several pages and I'm not sure if filter should be indicated on the "Threads Vizualizer" page above palette and time scale.
Comment 14 Eugene Chan CLA 2008-03-05 11:24:24 EST
(In reply to comment #13)
> Eugene, 
> do you think, that filter should be indicated on every page?
> E.g. Thread Viewer has several pages and I'm not sure if filter should be
> indicated on the "Threads Vizualizer" page above palette and time scale.
> 

Igor,

The changes should be applied to all views that support filter and currently show "(Filter: %filter_name%)" in the grey area. I would suggest to leave the grey area  and only remove the "(Filter: %filter_name%)" section in the grey area. The grey area contains process and host information which is useful for use case when users switch selection in profiling monitor view.
To answer your question. Since Thread Analysis view does not have filtering support. I don't think there is any need to add the filter text.
Comment 15 Igor Alelekov CLA 2008-03-21 03:24:16 EDT
Created attachment 93091 [details]
patch for the org.eclipse.hyades.trace.views
Comment 16 Igor Alelekov CLA 2008-03-21 03:47:49 EDT
Created attachment 93094 [details]
patch for the org.eclipse.tptp.trace.jvmti.client

Alex, could you check these two patches into CVS, since I still haven't write permition.
Comment 17 jkubasta CLA 2008-03-21 19:00:49 EDT
I committed Igor's two patches to Head so that this fix will be included in tonight's build.
Comment 18 Paul Slauenwhite CLA 2009-06-30 14:19:01 EDT
As of TPTP 4.6.0, TPTP is in maintenance mode and focusing on improving quality by resolving relevant enhancements/defects and increasing test coverage through test creation, automation, Build Verification Tests (BVTs), and expanded run-time execution. As part of the TPTP Bugzilla housecleaning process (see http://wiki.eclipse.org/Bugzilla_Housecleaning_Processes), this enhancement/defect is verified/closed by the Project Lead since this enhancement/defect has been resolved and unverified for more than 1 year and considered to be fixed. If this enhancement/defect is still unresolved and reproducible in the latest TPTP release (http://www.eclipse.org/tptp/home/downloads/), please re-open.