-
Notifications
You must be signed in to change notification settings - Fork 703
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
Delay load NavigationView's heirarchical chevron #6141
Conversation
…d. Fix a bug in AnimatedIcon where the AnimatedVisuals would be created twice on initial load.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -17,7 +17,7 @@ MicaController::~MicaController() | |||
// Workaround for null ref exception in Window::get_SystemBackdrop when the Window is shutting down. | |||
// GenerateRawElementProviderRuntimeId will trigger an exception caught below and thus prevent the | |||
// crashing target.SystemBackdrop() call. | |||
auto runtimeId = winrt::Automation::Peers::AutomationPeer::GenerateRawElementProviderRuntimeId(); | |||
auto const runtimeId = winrt::Automation::Peers::AutomationPeer::GenerateRawElementProviderRuntimeId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we saving this if we are not using the value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, I believe the answer is there is a cppguidelines checker static analysis has a rule that functions with a return value should be stored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, yeah makes sense. This won't make a big difference but should we then rename the variable to ignored to indicate that this is intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Handy links: |
🎉 Handy links: |
🎉 Handy links: |
🎉 Handy links: |
🎉 Handy links: |
The Chevron isn't needed for the majority of navigation view items but was being created regardless. This change makes it so the AnimatedIcon isn't loaded until it is actually needed.
Also fixes an issue in AnimatedIcon where the AnimatedVisual was being created twice.