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.

Sign up for GitHub

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

Jump to bottom

Upgrade to MUX 2.8 #15078

Merged
merged 26 commits into from
Apr 3, 2023
Merged

Upgrade to MUX 2.8 #15078

merged 26 commits into from
Apr 3, 2023

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Mar 31, 2023 •
edited
Loading

Updates the Terminal to Microsoft.UI.Xaml v2.8.

  • MUX 2.8 adds a dependency on WebView2, so we need to include that too.
  • See Ensure correct .props and .targets order in nuget package microsoft-ui-xaml#7574 for why we're adding the .props
  • The TabView thing:

    tl;dr: In >=MUX 2.7, we were updating our tab colors by doing a "Visual State Dance", as I called it. We'd manually change the TabViewItem's VisualState to one that it wasn't in, then change it back to the one it should be in. This seemingly re-applied the new values of the brushes. However in 2.8, this seemingly didn't work anymore!

    So instead, we do a "Theme Dance", like so:

      const auto& reqTheme = TabViewItem().RequestedTheme();
  TabViewItem().RequestedTheme(ElementTheme::Light);
  TabViewItem().RequestedTheme(ElementTheme::Dark);
  TabViewItem().RequestedTheme(reqTheme);

    This causes the ThemeResources to be re-evaluated to the new values.
    We never got to the root cause of why this seems different in 2.8. It literally makes no sense.

checklist

@zadjii-msft
This at least builds. Let's see what happens next
14ff89d
@zadjii-msft
for the sake of bookmarking: Add a manual copy of the TabView style
a9ca3da 
  this is a copy of the TabView style as of today's main,
  https://github.com/microsoft/microsoft-ui-xaml/blob/93e23c225c3f3a45878d8300426c6a45dc38c8aa/dev/TabView/TabView.xaml

  I'm gonna change this to see if we can fix this
@zadjii-msft
what the heck is going on
c80b6f0
@zadjii-msft
this is what karen said to try, but I can't even load XAML any more
feeaa67
@zadjii-msft
Using a -prerelease at least builds and runs for now
b3de7fe
@zadjii-msft
Some failed attempts
539e1cc
@zadjii-msft
Bunch of things that didn't work
8cd4b54
@zadjii-msft
shorthand
82f7766
@zadjii-msft
OH SHIT ONE OF THESE CRAZY IDEAS WORKED but I have no idea which, and…
ed5ed76 
… hovering doesn't work, but it's BETTER
@zadjii-msft
Narrow it down - this works too
0a8a079
@zadjii-msft
narrow it down more
fd467eb
@zadjii-msft
Bring the hover color back
cf96f88
@zadjii-msft
Didn't need these
e0ee0b8
@zadjii-msft
If this one's okay, are they all okay?
900b64f
@zadjii-msft
Honestly, do I even need custom styles at all?
dbd7c4b
@zadjii-msft
SERIOUSLY WE DIDN'T NEED ANY OF THIS
00603bf
@zadjii-msft
i am dead inside
e8bb5b7
@zadjii-msft
100%, this doesn't make sense, but I'll take what I can get
b957640
@zadjii-msft
I guess I didn't need that. Moving on.
f6c9728
@zadjii-msft
Off the -prerelease, to the main one. Let's see if CI likes it
9801a96
@zadjii-msft
Merge remote-tracking branch 'origin/main' into dev/migrie/b/mux-2.8.…
e20deb6 
…2-april-2023
@github-actions

This comment has been minimized.

Sign in to view
@zadjii-msft zadjii-msft mentioned this pull request Apr 3, 2023
Closed
@carlos-zamora carlos-zamora added the Needs-Second It's a PR that needs another sign-off label Apr 3, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Second It's a PR that needs another sign-off label Apr 3, 2023
@DHowett
Copy link
Member

DHowett commented Apr 3, 2023

Make sure you clean up the PR body!

@zadjii-msft
Copy link
Member Author

@DHowett Should we block while we wait on the WebView2 thing?

DHowett
DHowett requested changes Apr 3, 2023
View reviewed changes
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

just for a little.

dep/nuget/packages.config
@@ -8,7 +8,8 @@
<package id="Microsoft.VCRTForwarders.140" version="1.0.4" targetFramework="native" />
<package id="Microsoft.Internal.Windows.Terminal.ThemeHelpers" version="0.6.220404001" targetFramework="native" />
<package id="Microsoft.VisualStudio.Setup.Configuration.Native" version="2.3.2262" targetFramework="native" developmentDependency="true" />
<package id="Microsoft.UI.Xaml" version="2.7.3" targetFramework="native" />
<package id="Microsoft.UI.Xaml" version="2.8.2" targetFramework="native" />
<package id="Microsoft.Web.WebView2" version="1.0.1661.34" targetFramework="native" />
Copy link
Member

Choose a reason for hiding this comment

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

TODO Dustin: look at the content of this package and see if we can suppress the DLL being copied in.

Copy link
Member

Choose a reason for hiding this comment

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

Looked at, fixed.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 3, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Settings UI Anything specific to the SUI Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Apr 3, 2023
@DHowett DHowett enabled auto-merge (squash) April 3, 2023 22:10
@DHowett DHowett merged commit 17cf44f into main Apr 3, 2023
@DHowett DHowett deleted the dev/migrie/b/mux-2.8.2-april-2023 branch April 3, 2023 22:40
@zadjii-msft zadjii-msft linked an issue Apr 4, 2023 that may be closed by this pull request
Active tab rounded corner graphics misaligned/incorrect in the tab row when Windows Display Scale set to 150% (recommended default settings on 14" FHD laptops) #14905
Closed
@zadjii-msft zadjii-msft mentioned this pull request Apr 11, 2023
Merged
@ianjoneill ianjoneill mentioned this pull request Apr 12, 2023
Closed
@zadjii-msft zadjii-msft mentioned this pull request Apr 13, 2023
Merged
microsoft-github-policy-service bot pushed a commit that referenced this pull request Apr 17, 2023
@zadjii-msft
Fix reordering tabs mysteriously shuffling the actual backing tabs (#…
9b960bc 
…15178)

TL;DR: we stopped getting `TabView.TabItemsChanged`. This meant that the
tab view would change its apparent order, but we wouldn't change the
backing tab order.

I'm fixing this by grabbing the index of the tab that starts the drag,
and the index of the tab view item at the end of the drag, and using
that to reorder our backing list.

Closes #15121

Upstream microsoft/microsoft-ui-xaml#8388

Regressed in #15078 - I'm pretty confident about this, since I've got a
1.18.931 build of the Terminal with tear-out, but not MUX 2.8.
zadjii-msft added a commit that referenced this pull request Apr 20, 2023
@zadjii-msft
Remove a 1px gap under the tabs only visible at >150% (#15164)
2aefb30 
Set the padding to the default TabViewHeaderPadding (8,0,0,0), but with
-1 on the bottom. This prevents a small 1px gap that can appear on 150%
scale displays between the tab item and the content. The 1 on top helps
keep
the tab the correct relative height within the tab row.


Regressed in #15078 

See also MSFT:40692364
@zadjii-msft zadjii-msft mentioned this pull request Apr 20, 2023
Merged
zadjii-msft added a commit that referenced this pull request Apr 24, 2023
@zadjii-msft
Also do the VisualState dance on the tab item (#15217)
ee05307 
Just changing the Theme also doesn't seem to work by itself - there
seems to be a way for the tab to set the deselected foreground onto
itself as it becomes selected. If the mouse isn't over the tab, that can
result in mismatched fg/bg's

Regressed around #15078 

Closes #15184
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DHowett DHowett DHowett approved these changes

@lhecker lhecker lhecker approved these changes

@PankajBhojwani PankajBhojwani Awaiting requested review from PankajBhojwani

@carlos-zamora carlos-zamora Awaiting requested review from carlos-zamora

Assignees
No one assigned
Labels
Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
4 participants
@zadjii-msft @DHowett @lhecker @carlos-zamora