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

Fix issue with titlebar sample setting caption button background to the wrong color #1368

Merged
merged 10 commits into from
Oct 31, 2023
Merged

Fix issue with titlebar sample setting caption button background to the wrong color #1368

merged 10 commits into from
Oct 31, 2023

Conversation

marcelwgn
Copy link
Collaborator

@marcelwgn marcelwgn commented Sep 27, 2023 •
edited

Description

Closes #1355

Motivation and Context

This fixes the issues mentioned in #1355 and also the fact that we showed two titlebars in some cases.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@pratikone
Copy link
Contributor

Thanks for the PR. Could you verify if this issue still exists with newer builds of WInUI gallery (built from repo). With merger with appwindow titlebar, I think this won't be an issue. But I could be wrong so it is worth checking it out.

@marcelwgn
Squashed commit of the following:
4713920 
commit 1d23724
Author: Marcel Wagner <marcel.alex.wagner@outlook.com>
Date:   Wed Oct 11 02:31:01 2023 +0200

    Cleanup unnecessary code (microsoft#1299)

commit 20cd457
Author: Scott Jones <scottj1s@gmail.com>
Date:   Tue Oct 10 17:30:05 2023 -0700

    Update to WinAppSDK 1.4.2 and remove temporary workaround for including version info without dupe warnings (microsoft#1379)

commit c2fd30d
Author: Scott Jones <scottj1s@gmail.com>
Date:   Tue Oct 10 10:47:39 2023 -0700

    Set TrimMode explicitly back to partial for .NET 7 compat (microsoft#1378)

    See:

    https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trimming-options?pivots=dotnet-7-0

commit f72609f
Author: Pratik Anand <pratik.preet@gmail.com>
Date:   Tue Oct 3 12:29:10 2023 -0700

    Clickable Interactive control sample for custom titlebar (microsoft#1360)

    With 1.4 changes, WinUI 3 custom titlebar now uses appwindow titlebar + nonclientinputpointersource apis under the hood. This opens up new possibilities like allowing clickable interactive controls like textbox, button in the titlebar area, surrounded by draggable region on both left and the right sides.

    This code adds a sample to titlebar page which shows users how to create interactive controls in winui 3 titlebar. It also demonstrates the power of mixing and matching high level winui 3 custom titlebar apis and low level nonclient apis.

commit 5a96afc
Author: Andrew KeepCoding <andrewkeepcoding@gmail.com>
Date:   Thu Sep 28 19:31:59 2023 +0900

    Fix issue that crashes the app when navigating to SemanticZoomPage (microsoft#1363)

commit 9650e4e
Author: Marcel Wagner <marcel.alex.wagner@outlook.com>
Date:   Thu Sep 28 12:31:39 2023 +0200

    Switch .NET 7 (microsoft#1115)

commit 8f1b425
Author: Marcel Wagner <marcel.alex.wagner@outlook.com>
Date:   Thu Sep 28 12:31:22 2023 +0200

    First round of UWP link replacement (microsoft#1347)

    Co-authored-by: Bob Pulliam <bpulliam@microsoft.com>
@marcelwgn
Fix compiler errors
a8a8a84
@marcelwgn
Squashed commit of the following:
fef721c 
commit 1d23724
Author: Marcel Wagner <marcel.alex.wagner@outlook.com>
Date:   Wed Oct 11 02:31:01 2023 +0200

    Cleanup unnecessary code (microsoft#1299)

commit 20cd457
Author: Scott Jones <scottj1s@gmail.com>
Date:   Tue Oct 10 17:30:05 2023 -0700

    Update to WinAppSDK 1.4.2 and remove temporary workaround for including version info without dupe warnings (microsoft#1379)

commit c2fd30d
Author: Scott Jones <scottj1s@gmail.com>
Date:   Tue Oct 10 10:47:39 2023 -0700

    Set TrimMode explicitly back to partial for .NET 7 compat (microsoft#1378)

    See:

    https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trimming-options?pivots=dotnet-7-0

commit f72609f
Author: Pratik Anand <pratik.preet@gmail.com>
Date:   Tue Oct 3 12:29:10 2023 -0700

    Clickable Interactive control sample for custom titlebar (microsoft#1360)

    With 1.4 changes, WinUI 3 custom titlebar now uses appwindow titlebar + nonclientinputpointersource apis under the hood. This opens up new possibilities like allowing clickable interactive controls like textbox, button in the titlebar area, surrounded by draggable region on both left and the right sides.

    This code adds a sample to titlebar page which shows users how to create interactive controls in winui 3 titlebar. It also demonstrates the power of mixing and matching high level winui 3 custom titlebar apis and low level nonclient apis.

commit 5a96afc
Author: Andrew KeepCoding <andrewkeepcoding@gmail.com>
Date:   Thu Sep 28 19:31:59 2023 +0900

    Fix issue that crashes the app when navigating to SemanticZoomPage (microsoft#1363)

commit 9650e4e
Author: Marcel Wagner <marcel.alex.wagner@outlook.com>
Date:   Thu Sep 28 12:31:39 2023 +0200

    Switch .NET 7 (microsoft#1115)

commit 8f1b425
Author: Marcel Wagner <marcel.alex.wagner@outlook.com>
Date:   Thu Sep 28 12:31:22 2023 +0200

    First round of UWP link replacement (microsoft#1347)

    Co-authored-by: Bob Pulliam <bpulliam@microsoft.com>
@marcelwgn
Merge branch 'main' into dev/1355-titlebar-background
ec46ebd
@marcelwgn
Copy link
Collaborator Author

@pratikone, I'm afraid this PR is still relevant, the issues were still present when I tested.

karkarl
karkarl reviewed Oct 17, 2023
View reviewed changes
WinUIGallery/ControlPages/TitleBarPage.xaml.cs
{
TitleBarHelper.SetBackgroundColor(window, currentBgColor);
}
TitleBarHelper.SetForegroundColor(window, currentFgColor);
Copy link
Collaborator

Choose a reason for hiding this comment

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

SetForegroundColor

nit: Capitalize first letter and add period to comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm afraid I'm not sure which first letter to capitalize here 😅

pratikone
pratikone approved these changes Oct 17, 2023
View reviewed changes
Copy link
Contributor

@pratikone pratikone left a comment

Choose a reason for hiding this comment

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

looks good. thanks @karkarl for verifying the issue too

@marcelwgn marcelwgn mentioned this pull request Oct 26, 2023
Closed
@pratikone
Copy link
Contributor

/azp run

@marcelwgn marcelwgn merged commit 6f3c651 into microsoft:main Oct 31, 2023
2 checks passed
@marcelwgn marcelwgn deleted the dev/1355-titlebar-background branch October 31, 2023 19:39
@marcelwgn
Copy link
Collaborator Author

Thank you @pratikone for the CI rerun and your feedback on this PR!

karkarl pushed a commit that referenced this pull request Jul 15, 2024
@marcelwgn @karkarl
Fix issue with titlebar sample setting caption button background to t…
88148d1 
…he wrong color (#1368)

<!--- Provide a general summary of your changes in the Title above -->

## Description
<!--- Describe your changes in detail -->
Closes #1355 
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
This fixes the issues mentioned in #1355 and also the fact that we
showed two titlebars in some cases.
## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->

## Screenshots (if appropriate):

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karkarl karkarl karkarl left review comments

@pratikone pratikone pratikone approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Title Bar custom background & foreground colors not properly working
3 participants
@marcelwgn @pratikone @karkarl