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

Visual Tree restructure #8052

Open
amwx opened this issue Apr 27, 2022 · 2 comments
Open

Visual Tree restructure #8052

amwx opened this issue Apr 27, 2022 · 2 comments
Milestone

Comments

@amwx
Copy link
Contributor

amwx commented Apr 27, 2022

Is your feature request related to a problem? Please describe.
The current structure of the visual tree leads to some complications in certain scenarios. As part of my work to refactor the FocusManager, I've had to move it to the Controls project so the navigation logic knows about certain controls (primarily related to popups). All popups (windowed or overlay) need to be treated on the same focus scope as the TopLevel they belong to, so that finding the first focusable element should check popups first before the main app content. (To handle windowed, I'm registering them with the OverlayLayer to keep track of them - similar to how UWP has the PopupRoot). For overlay popups, we need to make sure focus never leaves the OverlayLayer by enforcing a "cycle" navigation. Focus should also never enter any of the other layers in VisualLayerManager (like the adorner layer) at least by usual focus navigation (it is probably still possible to force it programmatically). All of these cases require knowledge about controls not present in Avalonia.Base

Part of the problem comes from Window and PopupRoot being TopLevels and the fact that TopLevel is part of the VisualTree. I remember seeing somewhere there was a desire to make WindowBase not a TopLevel but of course I can't find it now. That should also be extended to PopupRoot.

Current VisualTree structure in Avalonia
[TopLevel]
-> VisualLayerManager
    -> ContentPresenter (main content)
    -> OverlayLayer
    -> AdornerLayer
...

This means that all events and the focus navigation logic all bubble up to the parent TopLevel since it sits at the root of everything, and the content of the app is evaluated first via the current tab navigation logic.

In UWP/WinUI, their tree is structured differently - abstracting the Window object from the rest of the tree (this is simplified)

[CoreWindow/AppWindow]
-> ContentRoot
-> FocusManager
-> InputManager
-> VisualTree
    > ContentRoot
         -> ScrollViewer/ContentPresenter -> Main app content
     -> PopupRoot

I don't think we need to explicitly follow UWP here, especially to keep any change like this from becoming a major headache to users. My thought is something like:

[TopLevel] (no longer part of visual tree)
-> FocusManager (already added to IInputRoot, so this is already done)
-> VisualLayerManager (also no longer part of visual tree)
    -> AppContent (visual tree root)
    -> PopupLayer (visual tree root)
etc...

To manage this, remove VisualLayerManager from the tree and have it implement something like IVisualTree which can exist in Avalonia.Base project. TopLevel can then use this to create the actual app window for display

public interface IVisualTree
{
    IVisual AppContent { get; }
    IPopupLayer PopupLayer { get; } // (We have PopupRoot control so can't call this PopupRoot like in UWP)
    [Other layers as needed]
}

public interface IPopupLayer
{
    IReadOnlyList<IPopup> RegisteredPopups { get; }
   
    // Have OverlayLayer implement this, if its a windowed popup, it only gets added to the 
    // list of registered popups. If it's overlay, register & add to Children collection
    // Called when popup opens
    void RegisterPopup(IPopup p);

    // Called when popup closes
    void UnregisterPopup(IPopup p);
}

// Add to Avalonia.Visuals namespace
public interface IPopup
{
    // Light dismiss/flyouts have should get focus precedence over app content
    bool IsLightDismissable { get; set; }
}

// Add IVisualTree to IInputRoot to let TopLevel implement it
public interface IInputRoot
{
    IVisualTree { get; }
}

FocusManager property was added to IInputRoot and addition of IVisualTree and other additions above would then give the focus manager everything it needs to function properly (since all visual roots are separated, and popups can be resolved without knowing about actual Popup control) and be moved back to Avalonia.Base project

In use for desktop:
[TopLevel/WindowRoot/etc.]
-> FocusManager
-> VisualLayerManager
-> [AppContent] Window (this keeps Window objects functional similar to WPF by keeping them the root of the tree, but still allowing them to be separated from other layers)
-> PopupLayer (OverlayLayer)
etc.

In a separate case here, I use the OverlayLayer to host the ContentDialog and TaskDialog I've added to FluentAvalonia. The one problem of the current structure means that when a dialog is open, all the events still bubble up to the TopLevel meaning technically it's possible to invoke a Hotkey/KeyBinding/AccessKey while the dialog is open (the same is probably true for popups too). Separating the OverlayLayer from the main app window would solve this problem too.

Of course this is just one solution, there may be better.

@amwx amwx mentioned this issue Apr 27, 2022
3 tasks
@danielmayost
Copy link
Contributor

I remember seeing somewhere there was a desire to make WindowBase not a TopLevel but of course I can't find it now. That should also be extended to PopupRoot.

#7063

@amwx
Copy link
Contributor Author

amwx commented Apr 29, 2022

Found another problem, not directly related to this, but probably could be addressed in the same scope. Overlay popup adorners are drawn at the incorrect bounds.

image

"Flyout Button1" is the focused element, but the adorner is drawing at the bottom of the popup. What is happening is when the adorned element is set TransformedBounds is used as reference for where to draw the adorner. Except this is in global (window) coordinates, but OverlayPopupHost has it's own VisualLayerManager so the coordinates in the popup's adorner layer don't match.

I think the best solution here (regardless of my proposal above) is to remove VisualLayerManager from OverlayPopupHost, and move the AdornerLayer z-index above the OverlayLayer so that overlay's are drawn in the same coordinate system/use the same adorner layer as the parent TopLevel/Window

@amwx amwx changed the title Visual Tree refactoring Visual Tree restructure Apr 29, 2022
@maxkatz6 maxkatz6 added this to the 12.0 milestone Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants