Conversation
zadjii-msft
left a comment
There was a problem hiding this comment.
Overall this looks pretty good, but I wanna block for discussion on passing the TabBase to the HeaderControl, so hopefully more of this can just be magic
| _CreateContextMenu(); | ||
|
|
||
| // Add an event handler for the header control to tell us when they want their title to change | ||
| _headerControl.HeaderTitleChanged([weakThis = get_weak()](auto&& title) { |
There was a problem hiding this comment.
Do we need to author a custom event (HeaderTitleChanged) for this? Since Title is observable, shouldn't we be able to do this with the automatic PropertyChanged event?
For ex, TerminalPage.cppL1143-1163, we'd probably do something similar:
_headerControl.PropertyChanged([weakThis=get_weak()](auto&&, const WUX::Data::PropertyChangedEventArgs& args) {
if (auto tab{ weakThis.get() })
{
if (args.PropertyName() == L"Title")
{
tab->SetTabText(title);
}
}
});There was a problem hiding this comment.
Ah I should rename this event - the purpose of this event is to let the hosting tab know that the user tried to change the title to x, and once the tab receives that event it tells the headerControl to change its title to x.
The reason for this is so its always the tab that has ownership over the title
(the event was named this way in the first iteration when the headerControl indeed just changed the title and simply wanted to let the tab know about it, now its more like it tells the tab/asks the tab's permission to change its title)
zadjii-msft
left a comment
There was a problem hiding this comment.
Yea, these are all nits. Thanks for finally biting the bullet on this 🙌
| <FontIcon x:Name="HeaderZoomIcon" | ||
| FontFamily="Segoe MDL2 Assets" | ||
| Visibility="Collapsed" | ||
| Glyph="" | ||
| FontSize="12" | ||
| Margin="0,0,8,0"/> | ||
| <TextBlock x:Name="HeaderTextBlock" | ||
| Visibility="Visible" | ||
| Text="{x:Bind Title, Mode=OneWay}"/> | ||
| <TextBox x:Name="HeaderRenamerTextBox" | ||
| Visibility="Collapsed" | ||
| MinHeight="0" | ||
| Padding="4,0,4,0" | ||
| Margin="0,-8,0,-8" |
There was a problem hiding this comment.
let's make sure this still works under accessibility. @carlos-zamora wanna help out?
|
Sorry, I hate to block for things i mostly called nits 😄 |
|
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
This PR makes the Header of TabViewItem a custom user control. ## Validation Steps Performed Manual testing Closes microsoft#8201
|
🎉 Handy links: |
This PR makes the Header of TabViewItem a custom user control.
Validation Steps Performed
Manual testing
Closes #8201