Skip to content

Conversation

@Reidea
Copy link
Contributor

@Reidea Reidea commented May 17, 2025

Fixed missing ItemsSource binding. The binding should be a property that accesses the items collection. Renamed it to Items.
The class name TabViewModel is misleading. Renamed to TabItemTemplate.

@thevortexcloud
Copy link
Contributor

thevortexcloud commented May 24, 2025

The class name TabViewModel is misleading. Renamed to TabItemTemplate.

How is it misleading? It is a view model? It's not a template. The template is defined in the view using the <DataTemplate>. If anything this change makes it misleading/wrong and sounds like a minor MVVM violation to me.

@Reidea
Copy link
Contributor Author

Reidea commented May 24, 2025

The class name TabViewModel is misleading. Renamed to TabItemTemplate.

How is it misleading? It is a view model? It's not a template.

Looks like you are right. In my understanding TabItemViewModel should always have TabItemView.axaml and is created only in such a bundle. This confuses me.

@thevortexcloud
Copy link
Contributor

thevortexcloud commented May 24, 2025

In my understanding TabItemViewModel should always have TabItemView.axaml

Generally, yes. But there is no requirement for that. The view model can be used with any view that knows how to handle it. There is nothing stopping you from instead hooking the tab item up to something like a HeaderedContentControl in one view and a tab control in another. This is one of the core principles of MVVM. Essentially the view is coupled to the view model but the view model is not coupled to the view.

@Reidea Reidea closed this May 24, 2025
@timunie timunie reopened this May 25, 2025
@timunie
Copy link
Contributor

timunie commented May 25, 2025

I still think there are good parts in this PR that I want to merge. What if we rename TabItemVieModel just to ItemViewModel? Other than that, I like the changes as they are closer to a real world usecase.

What do you think, @Reidea ?

@Reidea
Copy link
Contributor Author

Reidea commented May 25, 2025

What do you think, @Reidea ?

Good idea. I renamed it..

Copy link
Contributor

@timunie timunie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you for your contribution and patiance. Very much appreciate it.

@timunie timunie merged commit 8da1acc into AvaloniaUI:main May 25, 2025
2 checks passed
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.

3 participants