Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 349546 - [Restructuring] EMF Facet facetSet editor
Summary: [Restructuring] EMF Facet facetSet editor
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: EMF-Facet (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P4 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Nicolas Bros CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 338813 349574
Blocks: 349556
  Show dependency tree
 
Reported: 2011-06-16 07:39 EDT by Nicolas Guyomar CLA
Modified: 2020-05-01 11:26 EDT (History)
2 users (show)

See Also:
gdupe: juno+


Attachments
org.eclipse.emf.facet.efacet.edit.zip (60.26 KB, application/x-zip-compressed)
2011-06-16 08:20 EDT, Nicolas Guyomar CLA
nicolas.bros: iplog+
nicolas.bros: review+
Details
org.eclipse.emf.facet.efacet.editor.zip (21.05 KB, application/x-zip-compressed)
2011-06-16 08:21 EDT, Nicolas Guyomar CLA
nicolas.bros: iplog+
nicolas.bros: review+
Details
org.eclipse.emf.facet.efacet.ui.zip (71.47 KB, application/x-zip-compressed)
2011-06-16 08:46 EDT, Nicolas Guyomar CLA
nicolas.bros: iplog+
nicolas.bros: review+
Details
Patch for Bug 349546 xFriend (1.59 KB, patch)
2011-06-16 08:48 EDT, Nicolas Guyomar CLA
nicolas.bros: iplog+
Details | Diff
org.eclipse.emf.facet.efacet.doc.zip (163.52 KB, application/x-zip-compressed)
2011-06-16 09:43 EDT, Nicolas Guyomar CLA
nicolas.bros: iplog+
nicolas.bros: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Guyomar CLA 2011-06-16 07:39:03 EDT
Hi,

We need a facetSet editor in EMF Facet.

Regards,
Nicolas Guyomar
Comment 1 Nicolas Guyomar CLA 2011-06-16 08:20:48 EDT
Created attachment 198085 [details]
org.eclipse.emf.facet.efacet.edit.zip

Hi,

Please find attached plug-in org.eclipse.emf.facet.efacet.edit, needed to be able to edit facet elements in the editor.

(a) I, Nicolas Guyomar, wrote 100% of the code I've provided.
(b) I have the right to contribute the code to Eclipse.
(c) I contribute the content under the EPL.
(d) This contribution contains no Cryptography features.

Regards,
Nicolas Guyomar
Comment 2 Nicolas Guyomar CLA 2011-06-16 08:21:49 EDT
Created attachment 198086 [details]
org.eclipse.emf.facet.efacet.editor.zip

Hi,

Please find attached plug-in org.eclipse.emf.facet.efacet.editor which contains the first version of a basic facet editor.

(a) I, Nicolas Guyomar, wrote 100% of the code I've provided.
(b) I have the right to contribute the code to Eclipse.
(c) I contribute the content under the EPL.
(d) This contribution contains no Cryptography features.

Regards,
Nicolas Guyomar
Comment 3 Nicolas Guyomar CLA 2011-06-16 08:46:42 EDT
Created attachment 198089 [details]
org.eclipse.emf.facet.efacet.ui.zip

Hi,

Please find attached plug-in org.eclipse.emf.facet.efacet.ui which contains several wizards for facet management:
- AddFacetAttributeWizard
- AddFacetOperationParameterWizard
- AddFacetOperationWizard
- AddFacetReferenceWizard
- CreateFacetInFacetSetWizard
- CreateFacetSetWizard

Authors are Gregoire Dupe (commiter), Nicolas Bros (commiter) and myself.

(a) I, Nicolas Guyomar, wrote 100% of the code I've provided.
(b) I have the right to contribute the code to Eclipse.
(c) I contribute the content under the EPL.
(d) This contribution contains no Cryptography features.

Regards,
Nicolas Guyomar
Comment 4 Nicolas Guyomar CLA 2011-06-16 08:48:59 EDT
Created attachment 198090 [details]
Patch for Bug 349546 xFriend

Hi,

Last contribution org.eclipse.emf.facet.efacet.ui needs a new x-friend from org.eclipse.emf.facet.util.core plug-in, here is a patch to do so.

(a) I, Nicolas Guyomar, wrote 100% of the code I've provided.
(b) I have the right to contribute the code to Eclipse.
(c) I contribute the content under the EPL.
(d) This contribution contains no Cryptography features.

Regards,
Nicolas Guyomar
Comment 5 Nicolas Guyomar CLA 2011-06-16 09:43:36 EDT
Created attachment 198104 [details]
org.eclipse.emf.facet.efacet.doc.zip

Hi,

Please find attached a new documentation plug-in org.eclipse.emf.facet.efacet.doc about EMF Facet wizards architecture

(a) I, Nicolas Guyomar, wrote 100% of the code I've provided.
(b) I have the right to contribute the code to Eclipse.
(c) I contribute the content under the EPL.
(d) This contribution contains no Cryptography features.

Regards,
Nicolas Guyomar
Comment 6 Nicolas Bros CLA 2011-09-23 08:13:38 EDT
Comment on attachment 198104 [details]
org.eclipse.emf.facet.efacet.doc.zip

This is a contribution from an employee of Mia-Software, targeting a future
Juno release. The company has signed a Member Committer Agreement. The
contribution does not need a CQ (see bug 322327).

I have made the documentation project a plug-in, and committed it in revision 865.
Comment 7 Nicolas Bros CLA 2011-09-23 08:17:05 EDT
Comment on attachment 198085 [details]
org.eclipse.emf.facet.efacet.edit.zip

This is a contribution from an employee of Mia-Software, targeting a future
Juno release. The company has signed a Member Committer Agreement. The
contribution does not need a CQ (see bug 322327).

Committed in revision 868.
Comment 8 Nicolas Bros CLA 2011-09-23 08:17:28 EDT
Comment on attachment 198086 [details]
org.eclipse.emf.facet.efacet.editor.zip

This is a contribution from an employee of Mia-Software, targeting a future
Juno release. The company has signed a Member Committer Agreement. The
contribution does not need a CQ (see bug 322327).

Committed in revision 868.
Comment 9 Nicolas Bros CLA 2011-09-23 09:15:14 EDT
Comment on attachment 198089 [details]
org.eclipse.emf.facet.efacet.ui.zip

I validate this new project, but with reservations:

* First, from the UI point of view:

I see two different EMF Facet wizard categories (both named "EMF Facet")

the wizards are not ergonomic : 
- wizards open on top of other wizards : a wizard can open a modal dialog box to select an element, but should not open a second wizard on top if itself.
- context actions are not available in the context menu, but in the main toolbar
- labels are not always clear, and some groups don't have any label: the user doesn't know which information they are supposed to provide
- on some pages, double-clicking an item selects it and goes to the next page, whereas on other pages it doesn't
- the Java class name fields appear editable even though they are not.

Facet action buttons always appear (in a disabled state) in the toolbar of each Perspective. This pollutes the user interface with actions unneeded in most contexts.

These wizards violate many Eclipse User Interface Guidelines:

6.11: Fill the context menu with selection oriented commands.
The actions to create a Facet in a FacetSet, a FacetAttribute in a Facet, etc. should be accessible from the context menu.

9.3: Contribute actions to the window menu bar first, and then to the window toolbar if they will be frequently used.
All actions in the toolbar should also be accessible from the main menu

5.3: "Start the wizard with a prompt, not an error message." (http://www.eclipse.org/articles/Article-UI-Guidelines/Contents.html)
5.5: "Validate the wizard data in tab order.  Display a prompt when information is absent, and an error when information is invalid."
The EMF Facet wizards should not start with an error message.

2.10: Follow the specific size specifications for wizard graphics.
The existing EMF Facet wizard icons are ridiculously small, and most wizards are missing a banner image

5.8: "Use a Browse Button whenever an existing object is referenced in a wizard." 
(the browse buttons are labeled "Select" or "..." in these wizards)

5.8 Browse Buttons: "In the Browse Dialog, invalid choices should not appear."
The Java class browse dialog proposes every existing class, and just ignores the user selection when the class is not of the expected type.


* Now, from the source code point of view:

These composites are more general and should be put in org.eclipse.emf.facet.widgets.celleditors, and probably modified to implement AbstractCellEditorComposite:
org.eclipse.emf.facet.efacet.ui.internal.composites.BrowseComposite
org.eclipse.emf.facet.efacet.ui.internal.composites.FilteredElementSelectionComposite
and this one should go in org.eclipse.emf.facet.widgets.celleditors.ecore:
org.eclipse.emf.facet.efacet.ui.internal.composites.SelectETypeComposite

These controls are re-usable outside of the EFacet UI, and can be moved to org.eclipse.emf.facet.common.ui:
org.eclipse.emf.facet.efacet.ui.internal.controls.EClassifierSelectionControl
org.eclipse.emf.facet.efacet.ui.internal.controls.MetamodelSelectionControl

org.eclipse.emf.facet.efacet.ui.internal.wizards.pages.AbstractFacetWizardPage tries to do too much, and implements all the wizard pages in a single class, with booleans to filter out the unneeded parts.
It would be better if it followed the pattern used in org.eclipse.jdt.ui.wizards.NewTypeWizardPage, with all the create*Controls methods.

The extension point org.eclipse.emf.facet.efacet.ui.queryFactoryWizardPageRegistration only proposes a "managedQueryTypeName" id for the types of queries. This id is not defined as translatable, but is presented in the UI nevertheless.
This violates UI Guideline 5.7: Remove all programming message ID's from wizard text.
Comment 10 Nicolas Bros CLA 2011-09-23 09:18:29 EDT
Comment on attachment 198089 [details]
org.eclipse.emf.facet.efacet.ui.zip

This is a contribution from an employee of Mia-Software, targeting a future
Juno release. The company has signed a Member Committer Agreement. The
contribution does not need a CQ (see bug 322327).

Committed in revision 871.
Comment 11 Gregoire Dupe CLA 2011-11-24 07:42:34 EST
This contribution has introduced some warnings caused by useless import. I've fixed them.

Committed revision 1128.
Comment 12 Gregoire Dupe CLA 2012-05-23 12:12:47 EDT
This bug can be marked as fixed.
Comment 13 Gregoire Dupe CLA 2012-05-23 12:32:34 EDT
Sorry Vincent but this bug has to be closed for the IP log generation.