| Summary: | The selected post filter needs to be better indicated | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | amehrega | ||||||||||||||
| Component: | TPTP | Assignee: | Igor Alelekov <igor.alelekov> | ||||||||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||||||||
| Severity: | normal | ||||||||||||||||
| Priority: | P1 | CC: | analexee, ewchan, guru.nagarajan, jkubasta, ruslan.scherbakov | ||||||||||||||
| Version: | unspecified | Keywords: | plan | ||||||||||||||
| Target Milestone: | --- | ||||||||||||||||
| Hardware: | PC | ||||||||||||||||
| OS: | Windows XP | ||||||||||||||||
| Whiteboard: | closed460 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
amehrega
Created attachment 54974 [details]
Current view
Created attachment 54975 [details]
proposed changes
This will be addressed in 4.4 Perhaps it might be a good idea to remove the default post filter. It will be very confusing to novice users. Created attachment 65186 [details] Proposed view This proposal is slightly different than proposed in https://bugs.eclipse.org/bugs/attachment.cgi?id=54975 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. 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. 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). (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. Thanks Ruslan. Eugene, Ali - Can one of you drive this froward from here - targetting to 4.4 and/or applying to later versions. I agree with Eugene. The look and feel of all statistic views need to be consistent. I will have this targeted to future. 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. 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. (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. Created attachment 93091 [details]
patch for the org.eclipse.hyades.trace.views
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.
I committed Igor's two patches to Head so that this fix will be included in tonight's build. 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. |