View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0033528 | Lazarus | LCL | public | 2018-03-28 16:57 | 2018-09-19 12:12 |
Reporter | Serge Anvarov | Assigned To | Bart Broersma | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.9 (SVN) | ||||
Target Version | 1.8.6 | Fixed in Version | 1.8.6 | ||
Summary | 0033528: The Position property of the TFindDialog or TReplaceDialog components is ignored | ||||
Description | The Position, Left and Top properties of TFindDialog (or TReplaceDialog) are ignored. The dialog is always displayed in the center of the main form. In Delphi these properties are taken into account. | ||||
Steps To Reproduce | On an empty project, put the TFindDialog component and the TButton with event: procedure TForm1.Button1Click(Sender: TObject); begin FindDialog1.Position := Point(100, 100); FindDialog1.Execute; end; The same for the TReplaceDialog. | ||||
Tags | No tags attached. | ||||
Fixed in Revision | r59031, r59048 | ||||
LazTarget | 1.8.6 | ||||
Widgetset | |||||
Attached Files |
|
related to | 0034297 | closed | Bart Broersma | Issues with testing TFindDialog and TReplaceDialog in form designer |
|
finddialog.diff (1,613 bytes)
Index: lcl/include/finddialog.inc =================================================================== --- lcl/include/finddialog.inc (revision 57570) +++ lcl/include/finddialog.inc (working copy) @@ -81,7 +81,6 @@ BorderIcons := [biSystemMenu, biHelp]; Caption := 'Find'; //OnCreate := @FormCreate; - Position := poMainFormCenter; LCLVersion := '1.3'; @@ -520,9 +519,6 @@ HelpButton.OnClick := @HelpClick; CancelButton.OnClick := @CancelClick; PopupMode := pmAuto; - // Init local property with defaut value - FFormTop := top; - FFormLeft := Left; end; end; @@ -576,13 +572,13 @@ end; procedure TFindDialog.CalcPosition(aForm:Tform); -Var - MfBound : Trect; - begin - MfBound := Application.MainForm.BoundsRect; - FFormTop := MfBound.Top + (((MfBound.Bottom - MfBound.Top) - aForm.Height) Div 2); - FFormLeft := MfBound.Left + (((MfBound.Right - MfBound.Left) - aForm.Width) Div 2); + if (FFormLeft = 0) and (FFormTop = 0) then // Self.Position not set + begin + aForm.Position := poMainFormCenter; + FFormTop := aForm.Top; + FFormLeft := aForm.Left; + end; aForm.Top := FFormTop; aForm.Left := FFormLeft; end; Index: lcl/include/replacedialog.inc =================================================================== --- lcl/include/replacedialog.inc (revision 57570) +++ lcl/include/replacedialog.inc (working copy) @@ -73,7 +73,6 @@ AutoSize := True; BorderIcons := [biSystemMenu, biHelp]; Caption := 'Replace Text'; - Position := poMainFormCenter; LCLVersion := '1.3'; TextLabel := TLabel.Create(Self); |
|
Patch added |
|
(0,0) as indicator for not setting Left/Top? Is that how Delphi does it? B.t.w.: what about other TCommonDialog descendents? |
|
Yes (0,0) is default value for fields - InitInstance behavior. Delphi default is (-1,-1), but it uses WinAPI FindText function, witch has own default position. I do not know about TCommonDialog, because these fields (FFormTop, FFormLeft) are defined only in TFindDialog. |
|
(-1,-1) to me seems a better value to indicate no Top/Left is set, and then use poMainFormCenter. |
|
Please test and close if OK. Dialog is centered if both Top and Left < 0, which is wat Delphi 7 seems to do. |
|
I don't like your correction at all. Compare {bad} code [code] if (Self.Position.x < 0) and (Self.Position.y < 0) then begin MfBound := Application.MainForm.BoundsRect; FFormTop := MfBound.Top + (((MfBound.Bottom - MfBound.Top) - aForm.Height) Div 2); FFormLeft := MfBound.Left + (((MfBound.Right - MfBound.Left) - aForm.Width) Div 2); end else begin FFormLeft := Position.x; FFormTop := Position.y; end; [/code] with [code] if (FFormLeft < 0) and (FFormTop < 0) then begin aForm.Position := poMainFormCenter; FFormTop := aForm.Top; FFormLeft := aForm.Left; end; [/code] 1. if ... => 8 procedure calls 2. MfBound := ... => unsafe calculation (TForm do it better, with check for nil), additional var 3. FFormLeft := Position.x; => 4 procedure call FFormTop := Position.y; => 4 procedure call Withal in constructor [code] Position := Point(-1,-1); [/code] with 5 proc call and [code] FFormLeft := -1; FFormTop := -1 [/code] Let's take my correction, taking into account "-1". |
|
Unassigning. Your comments about {bad} code have nothing to do with the original issue. |
|
This is not a reason to do it carelessly. I added a thorough patch and it was ignored. |
|
While I can see your point about replacing Position := Point(-1,-1) with assignments to FFormTop/FFromLeft, the MFBound parts IMO should go inot a separate issue (and revision). Also, since this form reacts to user interaction, a few cycles doesn't really matter. Code readability does however. I'll commit the MFBound part seperately. I think that code predates the poMainFormCenter define? PS. The tone of your post just put me off. Hence I unassigned. (It's been a hard day at work here) |
|
Committed the second part of your fix (poMainFormCenter) in r59049. |
|
Please test and close if OK. |
|
I apologize for my tone. Thanks, it's all right. |
Date Modified | Username | Field | Change |
---|---|---|---|
2018-03-28 16:57 | Serge Anvarov | New Issue | |
2018-03-28 16:58 | Serge Anvarov | File Added: finddialog.diff | |
2018-03-28 16:58 | Serge Anvarov | Note Added: 0107444 | |
2018-03-28 17:23 | Bart Broersma | LazTarget | => - |
2018-03-28 17:23 | Bart Broersma | Note Added: 0107445 | |
2018-03-28 17:23 | Bart Broersma | Assigned To | => Bart Broersma |
2018-03-28 17:23 | Bart Broersma | Status | new => feedback |
2018-03-28 19:49 | Serge Anvarov | Note Added: 0107447 | |
2018-03-28 19:49 | Serge Anvarov | Status | feedback => assigned |
2018-03-28 19:50 | Serge Anvarov | Note Edited: 0107447 | View Revisions |
2018-03-28 23:12 | Bart Broersma | Note Added: 0107454 | |
2018-09-16 14:56 | Bart Broersma | Fixed in Revision | => r59031 |
2018-09-16 14:56 | Bart Broersma | LazTarget | - => 1.8.6 |
2018-09-16 14:56 | Bart Broersma | Note Added: 0110786 | |
2018-09-16 14:56 | Bart Broersma | Status | assigned => resolved |
2018-09-16 14:56 | Bart Broersma | Fixed in Version | => 1.8.6 |
2018-09-16 14:56 | Bart Broersma | Resolution | open => fixed |
2018-09-16 14:56 | Bart Broersma | Target Version | => 1.8.6 |
2018-09-17 17:28 | Serge Anvarov | Note Added: 0110827 | |
2018-09-17 17:28 | Serge Anvarov | Status | resolved => assigned |
2018-09-17 17:28 | Serge Anvarov | Resolution | fixed => reopened |
2018-09-17 20:58 | Bart Broersma | Note Added: 0110831 | |
2018-09-17 20:58 | Bart Broersma | Assigned To | Bart Broersma => |
2018-09-17 20:58 | Bart Broersma | Status | assigned => new |
2018-09-17 21:12 | Bart Broersma | Note Edited: 0110831 | View Revisions |
2018-09-17 21:46 | Serge Anvarov | Note Added: 0110836 | |
2018-09-17 22:01 | Bart Broersma | Note Added: 0110837 | |
2018-09-17 22:02 | Bart Broersma | Note Edited: 0110837 | View Revisions |
2018-09-17 22:02 | Bart Broersma | Assigned To | => Bart Broersma |
2018-09-17 22:02 | Bart Broersma | Status | new => assigned |
2018-09-17 22:07 | Bart Broersma | Fixed in Revision | r59031 => r59031, r59048 |
2018-09-17 22:16 | Bart Broersma | Note Added: 0110838 | |
2018-09-17 22:17 | Bart Broersma | Note Added: 0110839 | |
2018-09-17 22:17 | Bart Broersma | Status | assigned => resolved |
2018-09-17 22:17 | Bart Broersma | Resolution | reopened => fixed |
2018-09-18 16:42 | Serge Anvarov | Note Added: 0110850 | |
2018-09-18 16:42 | Serge Anvarov | Status | resolved => closed |
2018-09-19 12:12 | wp | Relationship added | related to 0034297 |