Comment 1 Idrissa Dieng 2011-04-08 07:44:39 EDT

Created attachment 192816 [details]
Allow the possibility of not showing group frame when creating the group content.

Comment 2 Stephan Eberle 2011-04-08 20:47:53 EDT

The creation of the frame (i.e., SWT Group widget) is repeated in each IGroup implementation:

if (suppressGroupFrame) {
			parentComposite = parent;
} else {
			Group fileSelectionGroup = new Group(parent, SWT.SHADOW_NONE);
			fileSelectionGroup.setText(groupName);
			parentComposite = fileSelectionGroup;
}

If would be better to refactor this part into AbstractGroup#createContent() and then call AbstractGroup#doCreateContent() with the old signature, i.e., without the additional suppressGroupFrame parameter.

Aside from that, please make sure that the method 

	void createContent(Composite parent, int numColumns);

gets added to the IGroup interface. Currently it is only on AbstractGroup.

Comment 3 Idrissa Dieng 2011-04-11 06:55:44 EDT

(In reply to comment #2)
> The creation of the frame (i.e., SWT Group widget) is repeated in each IGroup
> implementation:
> 
> if (suppressGroupFrame) {
>             parentComposite = parent;
> } else {
>             Group fileSelectionGroup = new Group(parent, SWT.SHADOW_NONE);
>             fileSelectionGroup.setText(groupName);
>             parentComposite = fileSelectionGroup;
> }
> 
> If would be better to refactor this part into AbstractGroup#createContent() and
> then call AbstractGroup#doCreateContent() with the old signature, i.e., without
> the additional suppressGroupFrame parameter.
> 
> Aside from that, please make sure that the method 
> 
>     void createContent(Composite parent, int numColumns);
> 
> gets added to the IGroup interface. Currently it is only on AbstractGroup.

Please refer to attach patch that include code refactor proposed by Stephan.

Comment 4 Idrissa Dieng 2011-04-11 06:58:25 EDT

Created attachment 192923 [details]
Refactor AbstractGroup#createContent() method

Comment 5 Idrissa Dieng 2011-04-12 10:46:28 EDT

Created attachment 193053 [details]
Refactor AbstractGroup#createContent() method and Add JavaDoc

Comment 6 Idrissa Dieng 2011-04-12 10:48:13 EDT

(In reply to comment #5)
> Created attachment 193053 [details]
> Refactor AbstractGroup#createContent() method and Add JavaDoc

Include code refactor proposed by Stephan (comment #2).

Comment 7 Stephan Eberle 2011-04-13 12:25:03 EDT

Fixed by applying proposed patch in slightly improved form: 
* Reworked AbstractGroup#createContent() methods
* Moved initialization of layout data on parent composite from AbstractGroup implementations to AbstractGroup#createContent()
* Applied common commenting style to single line comments

Comment 8 Balazs Grill 2021-07-14 02:15:56 EDT

Mass-closing Resolved tickets