Bugzilla – Full Text Bug Listing
| Summary: | Bugfixes and improvements | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | Sebastien Dubois <sebastien.dubois> |
| Component: | Mylyn | Assignee: | Sebastien Dubois <sebastien.dubois> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | alvaro.sanchez-leon, steffen.pingel |
| Version: | unspecified | ||
| Target Milestone: | 0.9 | ||
| Hardware: | All | ||
| OS: | Windows Vista | ||
| Whiteboard: | |||
|
Description
Sebastien Dubois
I noticed a few minor UI nits such as dialog controls missing borders on Linux, menu labels not using title case or R4E actions being contributed to all context menus which are not in accordance with the Eclipse user guidelines. If you want to get a sense of the guidelines I would highly recommend this document: http://wiki.eclipse.org/User_Interface_Guidelines (In reply to comment #1) > I noticed a few minor UI nits such as dialog controls missing borders on Linux, > menu labels not using title case or R4E actions being contributed to all > context menus which are not in accordance with the Eclipse user guidelines. If > you want to get a sense of the guidelines I would highly recommend this > document: http://wiki.eclipse.org/User_Interface_Guidelines Yes we haven't tested really on Linux so far (this is planned to start shortly though). As for the guidelines I'll have a look and modify the code accordingly. Please let me know if you find anything else. BTW I just pushed an improved version of the UI today with the latest fixes and improvements. (In reply to comment #2) > (In reply to comment #1) > > I noticed a few minor UI nits such as dialog controls missing borders on Linux, > > menu labels not using title case or R4E actions being contributed to all > > context menus which are not in accordance with the Eclipse user guidelines. If > > you want to get a sense of the guidelines I would highly recommend this > > document: http://wiki.eclipse.org/User_Interface_Guidelines > > Yes we haven't tested really on Linux so far (this is planned to start shortly > though). As for the guidelines I'll have a look and modify the code > accordingly. Please let me know if you find anything else. > > BTW I just pushed an improved version of the UI today with the latest fixes and > improvements. I updated to UI plugin to comply to the Eclipse UI guidelines (or at least my understanding of them:-). I'll do the testing and fixes on linux next. Feel free to check it out and let me know if you have any questions/comments. (In reply to comment #3) > (In reply to comment #2) > > (In reply to comment #1) > > > I noticed a few minor UI nits such as dialog controls missing borders on Linux, > > > menu labels not using title case or R4E actions being contributed to all > > > context menus which are not in accordance with the Eclipse user guidelines. If > > > you want to get a sense of the guidelines I would highly recommend this > > > document: http://wiki.eclipse.org/User_Interface_Guidelines > > > > Yes we haven't tested really on Linux so far (this is planned to start shortly > > though). As for the guidelines I'll have a look and modify the code > > accordingly. Please let me know if you find anything else. > > > > BTW I just pushed an improved version of the UI today with the latest fixes and > > improvements. > > I updated to UI plugin to comply to the Eclipse UI guidelines (or at least my > understanding of them:-). I'll do the testing and fixes on linux next. > > Feel free to check it out and let me know if you have any questions/comments. I tested the plugin in a Linux environment and I fixed some display bugs specific to Linux. Everything should be fine now. I still seem to get an R4E menu in almost every context menu with disabled menu items for adding review artifacts. Would it be possible to hide that unless it actually has enbled sub menu items? I was also wondering why R4E contributes a custom properties view which seems to be identical with the platform properties view? (In reply to comment #5) > I still seem to get an R4E menu in almost every context menu with disabled menu > items for adding review artifacts. Would it be possible to hide that unless it > actually has enbled sub menu items? > > I was also wondering why R4E contributes a custom properties view which seems > to be identical with the platform properties view? R4E context-menu commands are visible as follows: Find review items: visible when an IProject, IJavaProject, ICProject, IFolder, IPackageFragmentRoot or IPackageFragment is selected, regardless of the view. Add Review Item: visible when an IFile, ISourceReference (JDT or CDT) is selected, regardless of the view. Also when a selection is done in an editor (IEditorPart) Add Anomaly: Same as add review item. This is the intended design, so that people can select the commands on the appropriate element, regardless of the view (package explorer, navigator, outline etc.). New elements will be added as we support more coding environments (currently we only support JDT and CDT). The commands are disabled when the conditions to execute them are not currently met. Typically, this is when no review is open to host the command recipient. What could be done is to not show the R4E context menu when the no command is enabled (i.e. when no review is open). As for the new R4E properties view, which is very similar to the platform properties view, it is there so that the review navigator element properties are always visible for any element that is selected in the review navigator itself. For instance, the platform properties view is updated to other properties when the focus is set to another view e.g. any editor view, or navigator view etc. We want the properties view to always display the review navigator selected element properly, regardless of focus. This is why we created our own properties view. I can change this if there is a way to restrict the platform properties view to do what we want. (In reply to comment #6) > What could be done is to not show the R4E context menu when the no command is > enabled (i.e. when no review is open). Yes, that's critical to do. Otherwise everyone installing R4E will end up with an extra menu even when R4E is not used. > As for the new R4E properties view, which is very similar to the platform > properties view, it is there so that the review navigator element properties are > always visible for any element that is selected in the review navigator itself. > For instance, the platform properties view is updated to other properties when > the focus is set to another view e.g. any editor view, or navigator view etc. > We want the properties view to always display the review navigator selected > element properly, regardless of focus. This is why we created our own > properties view. I can change this if there is a way to restrict the platform > properties view to do what we want. You can pin the properties view to a selection. You can also create several instances of the view so with some tweaking you should be able to make the platform Properties view do what you want it to do. I am not sure that I understand the purpose of the R4E properties view. Why does it need to show the review item properties while the user navigates to other views or editors? (In reply to comment #7) > (In reply to comment #6) > > What could be done is to not show the R4E context menu when the no command is > > enabled (i.e. when no review is open). > > Yes, that's critical to do. Otherwise everyone installing R4E will end up with > an extra menu even when R4E is not used. > Ok I will implement that then. > > As for the new R4E properties view, which is very similar to the platform > > properties view, it is there so that the review navigator element properties > are > > always visible for any element that is selected in the review navigator > itself. > > For instance, the platform properties view is updated to other properties when > > the focus is set to another view e.g. any editor view, or navigator view etc. > > We want the properties view to always display the review navigator selected > > element properly, regardless of focus. This is why we created our own > > properties view. I can change this if there is a way to restrict the platform > > properties view to do what we want. > > You can pin the properties view to a selection. You can also create several > instances of the view so with some tweaking you should be able to make the > platform Properties view do what you want it to do. > > I am not sure that I understand the purpose of the R4E properties view. Why does > it need to show the review item properties while the user navigates to other > views or editors? Ok I will see what I can do. The purpose of the R4E properties view is to keep the focus on the review navigator's element properties even if you work with other views (e.g. an editor view with a file containing code). That way, for instance, you can still see an anomaly properties as you browse the code in an editor to interact with the code this anomaly is created on. (In reply to comment #8) > Ok I will see what I can do. The purpose of the R4E properties view is to keep > the focus on the review navigator's element properties even if you work with > other views (e.g. an editor view with a file containing code). That way, for > instance, you can still see an anomaly properties as you browse the code in an > editor to interact with the code this anomaly is created on. Sound like pinning the Properties editor for that purpose would work well. Have you otherwise considered adding a pane to the bottom of the Reviews to show the details of a selected anomaly? It would seem easier to me to see all review details in one view rather than managing two separate views in addition to the editor where code is browsed. (In reply to comment #9) > (In reply to comment #8) > > Ok I will see what I can do. The purpose of the R4E properties view is to keep > > the focus on the review navigator's element properties even if you work with > > other views (e.g. an editor view with a file containing code). That way, for > > instance, you can still see an anomaly properties as you browse the code in an > > editor to interact with the code this anomaly is created on. > > Sound like pinning the Properties editor for that purpose would work well. Have > you otherwise considered adding a pane to the bottom of the Reviews to show the > details of a selected anomaly? It would seem easier to me to see all review > details in one view rather than managing two separate views in addition to the > editor where code is browsed. Ok I just pushed the fix to hide the R4E context menu when not used. I will experiment with pinning the properties editor as you suggest. The reason why we chose to use tye properties view is because it gives us a nice re-usable mecanism to see the model data, especially using tabbed properties. We liked what we saw in EMF and wanted a similar look and feel. Also keep in mind that R4E will soon add a significant amount of data for the elements (for the formal and informal review types which are currently being implemented) and we wanted to keep the review navigator as a pure navigator view without overloading it. The drawback is the view coordinatiing as you noticed by we can live with that I believe. (In reply to comment #10) > The reason why we chose to use tye properties view is because it gives us a nice > re-usable mecanism to see the model data, especially using tabbed properties. How often does a user need to drill into the anomaly details while browsing through code? > We liked what we saw in EMF and wanted a similar look and feel. Also keep in > mind that R4E will soon add a significant amount of data for the elements (for > the formal and informal review types which are currently being implemented) and > we wanted to keep the review navigator as a pure navigator view without > overloading it. The drawback is the view coordinatiing as you noticed by we can > live with that I believe. I was thinking of a split view similar to JUnit that shows structure and detail but does not overload the tree. I am wondering if there is a concise way of presenting anomaly details in read-only mode. I understand the need for a sophisticated UI when creating anomalies but I the Properties view strikes me as very generic making it difficult to grasp relevant details. (In reply to comment #11) > (In reply to comment #10) > > The reason why we chose to use tye properties view is because it gives us a > nice > > re-usable mecanism to see the model data, especially using tabbed properties. > > How often does a user need to drill into the anomaly details while browsing > through code? This will happen as lot. For instance when the author of the code want to review the anomaly that was created by the reviewer, it is a must that he can see what the anomaly comments are. So there is definitively a case to show the anomaly detail while browsing through an editor window. > > > We liked what we saw in EMF and wanted a similar look and feel. Also keep in > > mind that R4E will soon add a significant amount of data for the elements (for > > the formal and informal review types which are currently being implemented) > and > > we wanted to keep the review navigator as a pure navigator view without > > overloading it. The drawback is the view coordinatiing as you noticed by we > can > > live with that I believe. > > I was thinking of a split view similar to JUnit that shows structure and detail > but does not overload the tree. That could be an option. > > I am wondering if there is a concise way of presenting anomaly details in > read-only mode. I understand the need for a sophisticated UI when creating > anomalies but I the Properties view strikes me as very generic making it > difficult to grasp relevant details. Yes it is generic but that's could also be a positive. Using the tabbed properties view, one can easily implement a nice way to show read-only and writable properties for any review data element. For now I am not planning to change anything on that area, as I have other urgent tasks to do (We do have an internal deadline here for R4E to comply to as well). I will experiment with pinning the properties view to the review navigator view and include it if it can be done quickly. If you want ,you can experiment with it and let me know if you find a more user-friendly way of displaying the info. Thanks for the comments! I investigated view pinning but unfortunately it doesn't do what we want. When you pin the properties view, it will display the properties for the currently selected element only, regardless of what is selected afterwards. This is not what we want. What we want is to have a properties view that takes its input only from our review navigator view. If the selection changes within the review navigator view, we want it to be reflected in our properties view. I did not find any other way to do this than to have our own custom properties view. I'll leave that aside for now, but let me know if you find anything. Thanks for looking into that. You might be able to make it work if you manage the state of the Properties view programmatically by forcing a selection on the view if the selection changes in the Review Navigator. Definitely not hight priority but I think it's something that we should discuss on an upcoming call. THis batch of improvements is implemented. Will re-open a new Task for follow-up later if needed |