Gui.cs: Layout issue in the Label.

[BDisp]

In my opinion I think that the LayoutSubviews () method should not be virtual because it is essential for the redisposition of all views and any overriden may forget to call the base. But for now I just put the Label to call the base method. What is your opinion?

[tig]

What will break if we do this? I think it sounds like a reasonable idea, but I don't know how much existing behavior depends on it.

[BDisp]

What will break if we do this? I think it sounds like a reasonable idea, but I don't know how much existing behavior depends on it.

I did a search and just found it on Labeland Dialogbut this one he calls the baseas it should be.

[tig]

It will break Dialog; The only way to remove it as a virtual and fix is to add another event to View (LayoutStarted). I just did this and will submit a PR for this issue. Ok?

[tig]

Actually, I spoke too soon. Adding a new event lets classes do their thing without relying on overriding, but if I change Layout to non-virtual I get an OOM error. This will require more investigation.

[tig]

In Dialog.cs

...
            LayoutStarted += (args) => {
                LayoutStartedHandler ();
            };

...


void LayoutStartedHandler ()
{
    int buttonsWidth = GetButtonsWidth ();

    int shiftLeft = Math.Max ((Bounds.Width - buttonsWidth) / 2 - 2, 0);
    for (int i = buttons.Count - 1; i >= 0; i--) {
        Button button = buttons [i];
        shiftLeft += button.Frame.Width + 1;
        button.X = Pos.AnchorEnd (shiftLeft);
        button.Y = Pos.AnchorEnd (1);
    }
}

In View.cs:

/// <summary>
/// Invoked when a view starts executing or when the dimensions of the view have changed, for example in
/// response to the container view or terminal resizing.
/// </summary>
/// <remarks>
/// Calls <see cref="OnLayoutComplete"/> (which raises the <see cref="LayoutComplete"/> event) before it returns.
/// </remarks>
public virtual void LayoutSubviews ()
{
    if (!layoutNeeded)
        return;

    Rect oldBounds = Bounds;
    OnLayoutStarted (new LayoutEventArgs () { OldBounds = oldBounds });

    viewText.Size = Bounds.Size;

    // Sort out the dependencies of the X, Y, Width, Height properties
    var nodes = new HashSet<View> ();
    var edges = new HashSet<(View, View)> ();

...

/// <summary>
/// Raises the <see cref="LayoutStarted"/> event. Called from  <see cref="LayoutSubviews"/> before any subviews have been laid out.
/// </summary>
internal virtual void OnLayoutStarted (LayoutEventArgs args)
{
    LayoutStarted?.Invoke (args);
}

/// <summary>
/// Fired after the Views's <see cref="LayoutSubviews"/> method has completed. 
/// </summary>
/// <remarks>
/// Subscribe to this event to perform tasks when the <see cref="View"/> has been resized or the layout has otherwise changed.
/// </remarks>
public Action<LayoutEventArgs> LayoutComplete;

/// <summary>
/// Raises the <see cref="LayoutComplete"/> event. Called from  <see cref="LayoutSubviews"/> before all sub-views have been laid out.
/// </summary>
internal virtual void OnLayoutComplete (LayoutEventArgs args)
{
    LayoutComplete?.Invoke (args);
}

[BDisp]

@tig really think it is the best solution. Use an event and don't override. Do it please. Thanks.

[BDisp]

I'll close my PR too.

[BDisp]

@tig I already tested the code above and it seems working well.

[tig]

Did you change it away from virtual? If I do it i get an out of memory error when starting. Don't have time to look at it right now...

[BDisp]

Affirmative @tig. I don't get any errors after removing the virtual from LayoutSubviews (). Perhaps it is another change that you have made that may be affecting this and not exactly because of this situation.

[BDisp]

@tig see it here https://github.com/BDisp/gui.cs/tree/massive-pr-refactored-text-rendering-alignment-word-wrap-hotkeys-all-now-work-with-unicode

[tig]

This will be fixed in #702