Bug 334147

Comment 1 Rüdiger Herrmann 2011-01-12 13:09:34 EST

Created attachment 186657 [details]
Corrects positioning of snippet 1 to 200

The majority of the changes set a Grid- or FillLayout on the main shell of the snippet

Comment 2 Felipe Heidrich 2011-01-12 17:14:59 EST

I see, the problem is: setting a layout and calling setLocation/setSize/setBounds is still not correct code (not setting a good example).

For example, the first snippet 100:
This part
	Shell shell = new Shell(display);
	shell.setBounds(10, 10, 200, 200);
	Text text = new Text(shell, SWT.MULTI);
	text.setBounds(10, 10, 150, 150);
should be replaced by

	Shell shell = new Shell(display);
	shell.setBounds(10, 10, 200, 200);
	shell.setLayout(new GridLayout());
	Text text = new Text(shell, SWT.MULTI);
	text.setLayoutData(new GridData(150, 150));

and that still change the behavior a bit (the 10, 10 location).

Comment 3 Rüdiger Herrmann 2011-01-12 18:45:02 EST

Sorry, this was not inteded. Of course, the calls to setBounds/setSize/setLocation must be replaced with appropriate calls to setLayoutData.
If you can live with the remaining change in behavior (i.e. the 10, 10 location), I will rework the patch accordingly.

Comment 4 Felipe Heidrich 2011-01-13 10:01:27 EST

(In reply to comment #3)
> Sorry, this was not inteded. Of course, the calls to
> setBounds/setSize/setLocation must be replaced with appropriate calls to
> setLayoutData.
> If you can live with the remaining change in behavior (i.e. the 10, 10
> location), I will rework the patch accordingly.

I'm not sure if this is an interesting change. But since you are doing the work and I will review it.

Maybe is better (less change to the code and no change in the behaviour) to call:
Rectangle clientArea = parent.getClientArea();
and add clientArea.x and clientArea.y to every x,y passed in for setLocation/setBounds method.

What do you think ?

Comment 5 Rüdiger Herrmann 2011-01-13 12:33:08 EST

(In reply to comment #4)
> [ ... ]
> Maybe is better (less change to the code and no change in the behaviour) to
> call:
> Rectangle clientArea = parent.getClientArea();
> and add clientArea.x and clientArea.y to every x,y passed in for
> setLocation/setBounds method.
> 
> What do you think ?
Admittedly, I started out with the getClientArea() approach on the first few snippets. Later on I went on to using layout managers as I thought that the snippets should also set a good example in terms of how to position widgets in SWT.
However, as we started out to fix the positioning and not to 'improve' the snippets, I would favor the getClientArea() approach that you suggested.
So, if you don't object, I will rework the patch to use getClientArea().

Comment 6 Felipe Heidrich 2011-01-13 13:41:00 EST

Sounds good to me.

Comment 7 Rüdiger Herrmann 2011-01-31 12:16:47 EST

Created attachment 187981 [details]
Corrects the positioning of snippet 1 to 99

The patch corrects the positioning by adding the client area offset when setLocation/setBounds is called.

Comment 8 Felipe Heidrich 2011-02-09 11:14:31 EST

Patched released. Thank you very much for time.

Comment 9 Rüdiger Herrmann 2011-02-09 12:59:40 EST

Created attachment 188607 [details]
Snippet 100 to 199: Corrects the positioning

In the same way as the previous patch, this one corrects the positioning by adding the client area offset when setLocation/setBounds is called.

Comment 10 Rüdiger Herrmann 2011-02-09 13:01:04 EST

I just provided a patch for the next chunk: snippet 100 to 199

Comment 11 Felipe Heidrich 2011-02-14 10:40:09 EST

(In reply to comment #10)
> I just provided a patch for the next chunk: snippet 100 to 199

Released, Thank you 

more to come ?

Comment 12 Rüdiger Herrmann 2011-02-14 12:28:41 EST

(In reply to comment #11)
> [ ... ]
> Released, Thank you 
> 
> more to come ?
Thanks for accepting the changes.
Yes, there is more to come (200 - max-snippet). I just thought smaller chunks were less scary ;) 
I will let you know when I attach the last patch.

Comment 13 Rüdiger Herrmann 2011-02-14 16:58:56 EST

Created attachment 188960 [details]
Corrects the positioning of snippet 200 til end

This should be the last patch. Again, it corrects the positioning by adding the client area offset when setLocation/setBounds is called.

Comment 14 Felipe Heidrich 2011-02-17 11:51:51 EST

Fixed in HEAD,

Thank you again!