-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Added links to user docs inside About popup #1887
Conversation
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 think the Resources change is the only one that's blocking me TBH
Should we make those link not selectable? |
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'm shocked that the ResourceLoader doesn't seem to care about the newline at the ends of those strings. This looks fine to me
src/cascadia/TerminalApp/App.cpp
Outdated
@@ -185,6 +184,9 @@ namespace winrt::TerminalApp::implementation | |||
auto resourceLoader = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); | |||
const auto title = resourceLoader.GetString(L"AboutTitleText"); | |||
const auto versionLabel = resourceLoader.GetString(L"VersionLabelText"); | |||
const auto gettingStartedLabel = resourceLoader.GetString(L"GettingStartedLabelText\n"); |
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.
Wait does this work with the newline at the end of the key?
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.
Thanks for cleaning all of those up.
Because he said I could and Kayla satisfied his requests.
Hello @miniksa! 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 (
|
OK Kayla didn't move the last URI. But I did. And if the CI is good, then this is good. So automerge. |
* added links into about section * added resources and aka.ms links * moved links to resources * Move the feedback URI into the resources too!
🎉 Handy links: |
Summary of the Pull Request
Added links to "Getting Started", "Documentation", and "Release Notes" in the About section.
References
#1576
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Clicked the links and they all work.
TODO
The hover color is too dark to read when using dark theme.