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.

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

Add link to GitHub repo on about flyout #1449

Merged
merged 3 commits into from
Dec 11, 2020
Merged

Add link to GitHub repo on about flyout #1449

merged 3 commits into from
Dec 11, 2020

Conversation

chaitanyamehta
Copy link
Contributor

@chaitanyamehta chaitanyamehta commented Dec 6, 2020

Fixes #582

Display link to the GitHub repo on the about flyout. Implementation is similar to link shown in currency converter mode when application is offline. (Offline. Please check your%HL%Network Settings%HL%)

Description of the changes:

  • Add learn more and contribute string in en-us resource.
  • Display link below feedback button in About Flyout.
  • Disable ScrollViewer.HorizontalScrollBarVisibility so that text wraps properly.

About Page

How changes were validated:

  • Manually tested.

@@ -28,6 +28,7 @@
<Style x:Key="AboutFlyoutPresenterStyle" TargetType="FlyoutPresenter">
<Setter Property="IsTabStop" Value="False"/>
<Setter Property="AutomationProperties.AccessibilityView" Value="Raw"/>
<Setter Property="ScrollViewer.HorizontalScrollBarVisibility" Value="Disabled"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary? There is no ScrollViewer in the AboutFlyout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default control template of FlyoutPresenter contains a ScrollViewer.

Control Template

This is what it looks like if the ScrollViewer property setter is removed:
About Page with Scroll

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok. That makes sense. Good work!

@greedyAI greedyAI self-assigned this Dec 10, 2020
@greedyAI greedyAI self-requested a review December 10, 2020 22:51
greedyAI
greedyAI previously approved these changes Dec 10, 2020
joseartrivera
joseartrivera previously approved these changes Dec 10, 2020
@grochocki
Copy link
Contributor

Perfect! Thanks again, @chaitanyamehta.

@grochocki grochocki merged commit c479378 into microsoft:master Dec 11, 2020
@chaitanyamehta chaitanyamehta deleted the github-link branch December 12, 2020 04:02
@Chips1234 Chips1234 mentioned this pull request Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add link to GitHub repo on about flyout
4 participants