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

TopLevels with Modal == false don't receive an event indicating their container is Loaded/Ready #1883

Closed
tig opened this issue Jul 23, 2022 · 3 comments · Fixed by #1884
Closed

Comments

@tig
Copy link
Collaborator

tig commented Jul 23, 2022

In Application.cs

image

And in Begin():

image

Neither TopLevel.OnReady or TopLevel.OnLoaded recurse contained TopLevels. They should.

image

Something like this:

		/// <summary>
		/// Called from <see cref="Application.Begin(Toplevel)"/> before the <see cref="Toplevel"/> is redraws for the first time.
		/// </summary>
		virtual public void OnLoaded ()
		{
			foreach (Toplevel tl in Subviews.Where (v => v is Toplevel)) {
				tl.OnLoaded ();
			}
			Loaded?.Invoke ();
		}
@BDisp
Copy link
Collaborator

BDisp commented Jul 23, 2022

Because they are added as a superview's subview, so the Loaded event will never be invoked. Since you do Top.Add (wizard); automatically the wizard view is a Top's subview. To the Loaded event being invoked then it have to do Application.Run (wizard); instead.

@tig
Copy link
Collaborator Author

tig commented Jul 23, 2022

Because they are added as a superview's subview, so the Loaded event will never be invoked. Since you do Top.Add (wizard); automatically the wizard view is a Top's subview. To the Loaded event being invoked then it have to do Application.Run (wizard); instead.

Sorry @BDisp , but I'm not following what you are saying. I think you are just stating why it is this Issue exists.

I have tested my proposed fix (see above) and it works as expected, alleviating the need for me to set the first step manually in the WizardAsView scenario.

In the Wizards scenario, the non-modal codepath creates the Wizard AFTER Application.Run(Top) has been called, and thus Loaded never fires even with my fix. So, in the PR to fix this Issue I am going to change the Wizard API docs to read:

image

Make sense?

@BDisp
Copy link
Collaborator

BDisp commented Jul 23, 2022

I think your fix to the OnLoaded may work if the Toplevel is added before the Application.Run. The API docs to explain why doesn't work if the Wizard is added after run makes sense. I only was justifying why the Loaded event of a Toplevel, which is a subview of another Toplevel, isn't invoked. Good work.

@tig tig closed this as completed in #1884 Jul 23, 2022
tig added a commit that referenced this issue Jul 23, 2022
Fixes #1883. Child TopLevel's now get Loaded/Ready events
BDisp pushed a commit to BDisp/Terminal.Gui that referenced this issue Jul 26, 2022
tig added a commit that referenced this issue Aug 1, 2022
* Fixes #1861. Border Title property is preferable than the Text.

* Fixes #1866. Bug when scrolling text and type in a TextView. (#1868)

* Some fixes for the WebConsole support. (#1865)

* Invoking NotifyStopRunState for all situations.

* Added Clicked property to support web console.

* Changing to MoveDown to stay always visible.

* Fixes #1849. Wizard as non-popup is broken (#1853)

* trying to make it work

* Fixes #1849. Wizard as non-modal doesn't work

* Fixes #1855. Window and Frame content view without the margin frame.

* Fixing layout of non-modal

* WizardSTep is now a FrameView

* Now use Modal = false to set visual style automatically

* Removed Controls as an explicit construct. Now just Add to WizardStep

Co-authored-by: BDisp <[email protected]>

* Update docs with keybindings, global key event and designer (#1869)

* Added docs on keybinding and global key event

* Added TerminalGuiDesigner to showcases/examples

* Regenerated Docs (#1870)

* Fixed cancel logic. Title now shows for non-modal. (#1871)

* Fixes #1874. API docs on github are broken. (#1875)

* Fixes #1874. API docs on github are broken.

* Rebuild with docfx 2.59.3.0 version.

* Fixes Wizard cancel logic and updates docs (#1878)

* Fixed cancel logic. Title now shows for non-modal.

* trying to fix docs

* trying to fix docs

* Fixes #1867. Use Undo and Redo commands with WordWrap enabled. (#1877)

* Updated docs; regeneraged docs (#1881)

* Added a 'Read Only' to the Editor scenario Format menu. (#1882)

* Fixes #1883. Toplevel now propogates Loaded & Ready events to child Toplevel views.

* Updated API doc theme. Added Wizard Sample

* Tweaked API docs format and content. Fixed build warnings.

* Fixes #1889. Docs broken after org move.

* Regen API docs

* Fixes readme links to API docs

* Avoiding breaking change.

* Fixes typos.

* Passing string.Empty to Title from the default constructor.

* Initializes title with string.Empty instead of null.

Co-authored-by: Tig Kindel <[email protected]>
Co-authored-by: Thomas Nind <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants