-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Feature - Container Queries #16846
base: master
Are you sure you want to change the base?
Feature - Container Queries #16846
Conversation
You can test this PR using the following package version. |
You can test this PR using the following package version. |
You can test this PR using the following package version. |
Should probably be |
Thinking more about it: if that's the case then can we just use the existing |
Names aren't unique in css. They probably couldn't use Class as it will conflict with their existing Class concept. An element can have multiple classes, but only 1 name, but multiple elements can have the same name |
That would be the better class to implement it for. |
You can test this PR using the following package version. |
3a36f78
to
5d58ebd
Compare
I really like to see functionality expanding in this area. We had lots of related conversations with the media query branch and this brings several ideas together nicely. However, I don't think we should follow CSS this closely here. Let me explain. Firstly we should note that this functionality (a subset) was implemented in WinUI using AdaptiveTriggers. The idea behind AdaptiveTriggers was fairly powerful -- if UWP at the time worked like WPF's Triggers this would have been nearly perfect. Avalonia doesn't have style triggers. I've originally lamented that as I think they are pretty simple to use in several cases. However, style selectors with pseudoclasses and property matching have proven pretty good for this as well covering the vast majority of use cases a trigger would have otherwise handled. So all is well on that front but we obviously can't use AdaptiveTrigger. My instinct at this point is simply to extend the selector syntax to handle visual tree navigation. I.e. We need a syntax that can query another control's properties in the tree. We already have precedence for this: The nth-child selector works this way. The item itself doesn't have enough information to know whether the style should be applied -- but it's container parent does. Because some prior art exists here I have to think it can be generalized without a major performance impact. Some other points:
Bringing all this together, why isn't it possible to have a syntax like the following in Selectors themselves:
or
These are not fully developed ideas but hopefully it gets my point across. The style selector syntax itself is certainly debatable. |
I could be wrong/misunderstanding you, but is that not what this is? https://docs.avaloniaui.net/docs/reference/styles/style-selector-syntax#by-property-match |
I'm not sure of how the selector idea would work in the current style system. The desired size of all controls are stored in properties that aren't AvaloniaProperties, i.e. |
Well, that is a bit different because it's only checking for property matches on the control itself -- NOT another control in the tree. nth-child does actually get information from elsewhere in the tree. However, you've given me a better idea for the concept and syntax.
Yes, this a clear place where not having Width/Height properties like WPF is an issue. I'll explain more. However, it might still be possible. Also note that I'm starting at a very high level and trying to design the best API first. Then implementation will need to be considered and design revised as needed. That's just the process I'm used to. I know you are quite a bit further along than I am on this issue though. So keeping with my main points from above:
Here is the new syntax proposal:
This API is still a stretch but it has some clear benefits:
That was only an example above where the default would be the first parent. Specifying a parent control by name is probably more appropriate in usage. |
'Width' and 'Height' does exist in Avalonia. But it doesn' represent the laid out dimensions, which is the most important information for queries, of a control. |
@emmauss As I said, I'm starting at a high level and designing the best API first. Implementation second. There are multiple options here:
|
But can't easily combine the two? EG
That is how you typically do stuff in CSS. It does lead to some very verbose queries though.
Avalonia already sort of has this. It's just hidden under the |
I really don't like this. I've been against putting arbitrary expressions in selectors quite a few times (e.g. people have asked for selectors that can query properties on the data context). These are the reasons:
We already have a way to create layouts with complex logic: code-behind. I know people have some sort of dogmatic aversion to code-behind, but code is the place for such logic, not a DSL shoved into a syntax that was never intended to support it. People complain the Rx is hard to understand, but at least it's C# - here you have the equivalent but with a custom syntax and no debugging tools. |
CSS has been slowly moving in that direction. It went from being a simple text file that describes visuals to being basically a very simplified specialised programming language. People however keep taking it further with things like preprocessors that add actual programming concepts like control statements. I can't say I am a fan of it. In the context of XAML you are better off just using some sort of calculated property at that point. |
Please keep in mind I'm approaching this from a different viewpoint. Rather than trying to integrate a completely new concept: Container Query. I'm trying to fit the functionality into what we already have. As noted above, the core of this functionality is related to nth-child (parent properties used in selectors) and property value matching. It's a short extension from there to use ANY arbitrary parent control properties. The complexity is trying to find a syntax that works for the less-than and greater-than constraints as well as where do we get Width/Height from considering the deviations in Avalonia's design in this area.
As one who probably puts too much in code-behind and never jumped on the 100% MVVM bandwagon I agree with you more than you think. However, a few points:
If devs want to make their own eyes bleed they ALWAYS have that ability no matter what a framework does. I would never try to design around the least capable developer and reduce the feature set for everyone else. Besides, the person that ultimately would have to deal with such issues is... the developer or company that allows that in the first place. It doesn't affect anyone else. Whenever the person responsible is also the person accountable things have a way of working themselves out.
Well, we don't actually need full expressions (I did say I might be getting too creative). However, the subset to support less-than and greater-than is little different in cost than what is required for container queries implemented another way. I will say though simple arithmetic with CONSTANT values (as opposed to adding properties from multiple controls) will be much less of a performance concern. If we eventually supported binding to, as a contrived example, Height of one control and then added +10 to that value with a simple arithmetic expression there would be negligible performance impact and no more circular dependency than we already have. Bringing this back on topic: Does anyone else see my viewpoint? Trying to integrate this functionality into selectors. We are already part of the way there with how selectors currently function. I have no commitment to any specific selector syntax. I was just throwing out some ideas. I'm sure people have better ideas for syntax that could be more acceptable. Personally, I don't really like the ContainerQuery syntax specified above. I think it's overly specific when it should be better generalized and integrated with ideas we already have. |
5d58ebd
to
bb68d7b
Compare
/// <summary> | ||
/// Defines the interface for a container | ||
/// </summary> | ||
public interface IContainer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs to be a public API at least yet
|
||
double width, height; | ||
|
||
if (isContainer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that this logic should be bypassed for TopLevels and TopLevel should always implicitly be a SizeAndHeight container.
@@ -0,0 +1,36 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we somehow fit this demo into ControlCatalog? I'd really prefer to not have yet another set of projects in the solution.
I understand that it does require a separate TopLevel and we only have one on mobile, but we can probably just switch the MainView on a button click or something.
|
||
namespace Avalonia.Platform | ||
{ | ||
public class VisualQueryProvider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be public
/// <summary> | ||
/// Defines a container. | ||
/// </summary> | ||
public class Container : StyleBase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name feels to be too generic and is likely to cause name clashes later. We already have IContainer that means a completely different thing.
src/Avalonia.Base/Styling/Query.cs
Outdated
/// <summary> | ||
/// A query in a <see cref="Container"/>. | ||
/// </summary> | ||
public abstract class Query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, naming feels to be too generic. Maybe StyleQuery?
|
||
namespace Avalonia.Controls | ||
{ | ||
/// <summary> | ||
/// Base class for controls which decorate a single child control. | ||
/// </summary> | ||
public class Decorator : Control | ||
public class Decorator : Control, IContainer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The choice to mark controls as possible containers seems to be rather arbitrary.
What was the logic behind choosing Decorator, ContentControl and ContentPresenter? Why do we have both ContentControl and ContentPresenter as containers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are chosen because they are expected to have 1 child, and we can predict how it measures, thus makes more sense as containers. As for ContentControl and Presenter, that ContentPresenter should be the one container, so it is a mistake to make ControlControl one. I'll update it.
We could make all Controls be a potential container, but that won't make sense for controls that do not expect content, like Calendar and NumberUpDown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, even if a Control implements IContainer, it doesn't implicitly become a target for queries, as its ContainerType would be Normal. So ContentControl should be an IContainer. ContentPresenter is made to be an IContainer so allow users to make customer controls that can act as a container without implementing IContainer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the implementation is generic enough as-is. I see no reason a container query can't target ANY control. We can already opt-out by default with type Normal.
Keep in mind that the design of XAML is nesting of controls. That means this should be opt-out rather than opt-in. IMO the controls where it just never makes sense should have a mechanism to disable the container query provided by Control base by overriding the type.
src/Avalonia.Controls/WindowBase.cs
Outdated
@@ -284,6 +283,11 @@ protected override Size MeasureCore(Size availableSize) | |||
|
|||
var constraint = LayoutHelper.ApplyLayoutConstraints(this, availableSize); | |||
|
|||
if (this is Styling.IContainer container) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this condition is currently always true since WindowBase is a ContentControl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, why is it in WindowBase and not in TopLevel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TopLevel doesn't have any special MeasureCore implemention, relying on the base implementation in Layoutable and the override in WindowBase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- TopLevels should always act as Width+Height containers for satisfying media queries
- TopLevels should never be affected by the new Measure logic in Layoutable
- Manually setting ContainerType/ContainerName on a TopLevel is invalid and should trigger an exception
_argument = argument; | ||
} | ||
|
||
protected override bool EvaluateIsActive() => (CurrentContainer?.ContainerType == Layout.ContainerType.Width || CurrentContainer?.ContainerType == Layout.ContainerType.WidthAndHeight) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if container type changes at runtime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wouldn't evaluate as long as it isn't a container type that applies to the dimension.
public enum ContainerType | ||
{ | ||
/// <summary> | ||
/// The container will not be queries for any container size queries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix: The container will not be queried
for any container size queries.
better: The container is not included in any size queries.
Normal, | ||
|
||
/// <summary> | ||
/// The container can be queried for container size queries for width. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better: The container size can be queried for width.
/// <summary> | ||
/// The can be queried for container size queries for height. | ||
/// </summary> | ||
Height, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better: The container size can be queried for height.
Height, | ||
|
||
/// <summary> | ||
/// The can be queried for container size queries for both width and height. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better: The container size can be queried for width and height.
/// <summary> | ||
/// The can be queried for container size queries for both width and height. | ||
/// </summary> | ||
WidthAndHeight |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Several questions here:
- Why did you deviate from CSS? Just have a
Size
entry that is for everything Height and Width related - Why are Width and Height entries even provided?
- As far as I can tell there is no performance impact for both height and width (rather than just one)
- Under what circumstances would a container only have a height or width? It's always just a size we should care about, right?
- This is unnecessary complexity as far as I can tell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I responded earlier, CSS has Size and InlineSize as ContainerTypes. We do not have a concept of inline in Avalonia. Just having one size type would prevent containers from growing in size in cases where its preferable, like in ScrollViewers.
In CSS, InlineSize corresponds to Width in normal layouts, and this allows the Height of the container to grow to fit the content, but will fix the Width on the Container to what the element's parent provides. Size type prevents the Container from growing in both dimensions.
To preserve this behavior while giving the user a choice in which dimension to limit, Width, Height and WidthAndHeight are provided as ContainerType. This isn't unfamiliar to Avalonia users, as Window has SizeToContent enum with the same names.
set => SetValue(ContainerTypeProperty, value); | ||
} | ||
|
||
public string? ContainerName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is container name even different from control names? Unlike CSS we can name everything already, right? We should re-use the existing name property I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSS also has a unique element identifier called id( represented by the #). That corresponds to Name here in Avalonia. If they thought classes and ids would have been sufficient to identify containers, they wouldn't have the container-name
property for their containment feature.
Container name provides a mix between classes and ids/names. Unlike classes, you can only have 1 container name on a container, but unlike names, you can have multiple containers in the visual tree that have the same container. Containers are just "zones" in the view where you want to activate styles based on a query. This concept is beneficial to us, as we try to improve layout on browsers and mobile devices.
Well, no one wants to discuss different ideas for implementation here (extending selectors themselves) so I guess I'll just comment on the direction you are already going. Can I request in the future you have a discussion for feature additions like this BEFORE opening a PR with an implementation? It would be great for the community to discuss ideas and perhaps even vote on direction. This is all decided behind closed doors right now. Personally, I find things like this interesting to think about and I'm sure others do as well. |
Actually, nothing was decided behind closed doors - Emmanuel had an idea of how it should work, wrote the feature and opened a PR.
As you can probably appreciate, it's hard to come up with a useful spec without at least a prototype implementation. Having said that, now the feasibility has been proved, it might be a good time to open a discussion/issue to discuss the design. We should probably mark such PRs as "RFC" or something in future. |
…ake IContainer internal
I've opened an issue to discuss this. #16949 |
I overall agree because keeping XML-compatibility forces syntax into an awkward direction, but moving one inner bit to code-behind necessitates moving a lot more into code. If we could define converters or static methods (with parameters) in C# for the logic and reference them in selectors (or even as a custom selector), then the impact from necessarily splitting styling definitions into multiple approaches (C# and XAML) is minimized. |
You can test this PR using the following package version. |
You can test this PR using the following package version. |
What does the pull request do?
Container Queries allows styles to be activated for control based on the size of an ancestor, which acts as a container. This feature is similar to css's container queries.
A container can be defined by setting the
ContainerType
property.ContainerType
is an enum with the following values.Currently only
ContentControl
andBorder
support acting as a container. To add container behavior for custom controls that do not implement these two controls, implement theAvalonia.Styling.IContainer
interface.Queries are treated as selectors for the container, and are evaluated when the container size changes. Take the following use case
When ContentControl size changes, it triggers a reevaluation of any ContainerQuery attached to it. If the ContentControl's width is at least 400px, the styles in the Query are activated and the Border.b style is applied to any child that matches that selector.
Supported queries include;
A container query can specify which container to attach to by setting the
Container
property. The container will also need it'sContainerName
property to be set.Only the ContainerQuery matching the C1 container name will be evaluated.
Layout
Container queries activates styles based on the size of a visual. The styles applied can change the size of the container, so the container size should remain constant during the whole layout pass.
At the beginning of the Measure pass, the size constraints of the container, i.e. the maximum size the container can be with respect to it's set Width,Height, Min and Max dimension are sent for evaluation before children are measured.
Measure proceeds as normal, with DesiredSize set according to the follow after children are measured.
For an example, in the use case above the ContentControl queries for Width, so if the Width evaluated is PositiveInfinity, the measured width of its content will be used as the DesiredSize.Width. If it 700px, the DesiredSize.Width will be 700. The DesireSize.Height will be the height measure from the children in both cases.
This is a partial rewrite of #7938
What is the current behavior?
What is the updated/expected behavior with this PR?
How was the solution implemented (if it's not obvious)?
Checklist
Breaking changes
Obsoletions / Deprecations
Fixed issues
Fixes #16949