Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Jvrollout realigning controls on collapse/expand #125

Merged
merged 3 commits into from May 24, 2019

Conversation

Projects
None yet
3 participants
@MHumm
Copy link
Contributor

commented May 16, 2019

Contents of pull request #121 redone, as the other branch was deleted too early.
Fix for Mantis issue 6332.
http://issuetracker.delphi-jedi.org/view.php?id=6332

Markus Humm
Contents of pull request #121 redone, as the other branch was deleted…
… too early.

Prevent Jvrollout realigning controls on collapse/expand
@baka0815
Copy link

left a comment

Why did you change the lines around 996? You didn't do this in your first pr where you just put a try..finally around the DisableAlign and EnableAlign in the if Parent = FTopForm then case.
You're now disabling the align on a broader scope - why is that?

begin
FTopForm.Width := FAWidth;
FOldWidthHeight.X := FAWidth;
FTopForm.DisableAlign;

This comment has been minimized.

Copy link
@baka0815

baka0815 May 17, 2019

This DisableAlign is unnecessary when you're doing it in line 996 now.

FTopForm.Width := FAWidth;
FOldWidthHeight.X := FAWidth;
end;
FTopForm.EnableAlign;

This comment has been minimized.

Copy link
@baka0815

baka0815 May 17, 2019

Now you have this EnableAlign and the one some lines later (around 1034) in your finally block.

This comment has been minimized.

Copy link
@MHumm

MHumm May 18, 2019

Author Contributor

Ok, I fixed that now. Redoing the pr was obviously done a bit in a hurry. Sorry. I really hope it is good enough for getting merged in now!

@baka0815
Copy link

left a comment

Yep, this looks much better!

TWinControl(Controls[I]).Visible := False;
end;
DisableAlign;

This comment has been minimized.

Copy link
@obones

obones May 22, 2019

Member

To make it clearer that the DisableAlign is protected by the try..finally block, this empty line should be removed

TWinControl(FChildControlVisibility.Objects[I]).Visible := True;
FreeAndNil(FChildControlVisibility);
DisableAlign;

This comment has been minimized.

Copy link
@obones

obones May 22, 2019

Member

Same remark here

Markus Humm
Removed empty lines to make it clearer that the try/except block prot…
…ects the DisableAlignCall. Requested by Obones
@MHumm

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

Made the requested changes and comited them.

@obones

obones approved these changes May 24, 2019

@obones obones merged commit 5c43dd3 into project-jedi:master May 24, 2019

@MHumm MHumm deleted the MHumm:Mantis6332_JvRollout_realignment branch May 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.