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

Made TabControl observe changes which affect SelectedContent/Template, plus various analyzer warning fixes #11366

Conversation

TomEdwardsEnscape
Copy link
Contributor

@TomEdwardsEnscape TomEdwardsEnscape commented May 14, 2023

While fixing analyzer warnings, I noticed that TabControl and TabItems only synchronise content and content templates when the selected item changes. I switched them to bindings/observables, so that changes are applied immediately, e.g. in response to a style trigger.

I also noticed that TabControl.ContentTemplate was either not used at all (for a user-provided TabItem) or always used (for a generated TabItem). WPF evaluates both every time, and TabItem.ContentTemplate wins if not null. This PR introduces the same precedence rules.

The PR also contains the set of fixes for / suppressions of analyzer warnings that I originally set out to make, plus an update to Avalonia.Desktop.slnf which corrects the paths to some projects.

TabControl specifics

TabItem.TabStripPlacement is now a DirectProperty. I applied the same pattern that I used in ScrollBar to subscribe to TabControl.TabStripPlacement and forward the value. The value also is now nullable, so won't incorrectly return Dock.Left when there isn't a TabControl around.

TabControl.SelectedContent and TabControl.SelectedContentTemplate are now direct and read-only. These previously had analyzer suppression attributes saying that they were "supposed to be a styled readonly property", but this doesn't make sense, as they are set with local priority by TabControl and thus can never be styled.

Additionally, both of those properties are now set by observable subscribers. This means that changes to TabControl.ContentTemplate, TabItem.ContentTemplate, and TabItem.Content are now reflected immediately, instead of when the selected tab next changes.

TabControl.UpdateHeader

I didn't change this method despite the warnings it is emitting. It's a weird feature and I don't know what to do with it. Is it really desirable to sometimes automatically set the header when the content changes?

I think we should delete this code, and let users create their own attached behaviour if they actually want the functionality.

Breaking changes

  • Some properties have switched from styled to direct.
  • TabControl.ContentTemplate is now used if TabItem.ContentTemplate is null. This matches WPF.

…the selected tab changes

Flipped ContentTemplate priority to match WPF, where TabItem's template wins over TabControl's
Made various properties read-only DirectProperty
Updated desktop solution filter
@TomEdwardsEnscape TomEdwardsEnscape changed the title Made TabControl observe changes to SelectedContent/Template, plus various analyzer warning fixes Made TabControl observe changes which affect SelectedContent/Template, plus various analyzer warning fixes May 14, 2023
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0034716-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@Gillibald
Copy link
Contributor

if (Header == null)
this method doesn't look symmetrical. In case Header is null there is a check for IHeader for non null Header there isn't.

@grokys grokys requested a review from Gillibald May 15, 2023 10:07
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0034914-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@Mrxx99
Copy link
Sponsor Contributor

Mrxx99 commented May 17, 2023

Does this mean TabStripPlacement can't be set in a style? And why the limitation to DirectProperty?

@TomEdwardsEnscape
Copy link
Contributor Author

TabControl.TabStripPlacement can be set in a style. TabItem.TabStripPlacement just forwards the value of the controlling TabControl, for convenience when writing TabItem templates.

@robloo
Copy link
Contributor

robloo commented May 22, 2023

Just ran across TabControl not working with a custom ContentTemplate. Hopefully this makes it in for RC otherwise TabControl doesn't work with MVVM and dynamic tab items set with ItemsSource.

@grokys
Copy link
Member

grokys commented May 22, 2023

@Gillibald ping.

@Gillibald Gillibald added this pull request to the merge queue May 23, 2023
Merged via the queue into AvaloniaUI:master with commit 6533972 May 23, 2023
@TomEdwardsEnscape TomEdwardsEnscape deleted the fixes/tabitem-contenttemplate-analyzer-warnings branch May 23, 2023 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants