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

[UX] Adding links to control source code #1411

Merged
merged 6 commits into from
Feb 13, 2024
Merged

Conversation

niels9001
Copy link
Collaborator

Description

This PR adds a direct link to the WinUI 3 control source code (addressing: #1388):

  • A path to a control folder or XAML file (when the control is shipped with the OS) are defined in the ControlDataInfo.json
  • On the item page, a link is shown (when available) in the Source flyout. If not, the button is hidden.

This makes it easy for developers to quickly go to the source code / default XAML file of a control.

When a path is configured in ControlDataInfo.json:
image

the link navigates to: https://github.com/microsoft/microsoft-ui-xaml/tree/winui3/release/1.4-stable/controls/dev//ParallaxView

If no link is configured, the button is hidden:
image

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)

Copy link
Collaborator

@marcelwgn marcelwgn left a comment

Choose a reason for hiding this comment

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

I feel like the spacing on the flyout for the source code links seems a bit off, but I don't really know why. @niels9001 what are your thoughs on maybe increasing padding and alignment?

niels9001 and others added 3 commits December 21, 2023 23:37
Co-authored-by: Jay <65828559+Jay-o-Way@users.noreply.github.com>
Co-authored-by: Jay <65828559+Jay-o-Way@users.noreply.github.com>
@niels9001
Copy link
Collaborator Author

I feel like the spacing on the flyout for the source code links seems a bit off, but I don't really know why. @niels9001 what are your thoughs on maybe increasing padding and alignment?

@chingucoding I've tweaked the margins/paddings a bit of both flyouts, so they are consistent with a typical Flyout (16px inner padding). The HyperlinkButton adds additional whitepace that makes things unaligned. Should be fixed now:

Docs flyout:
image

Source flyout:
image

Default flyout:
image

"Title": "WinUI Theme Resources (Github)",
"Uri": "https://github.com/microsoft/microsoft-ui-xaml/blob/main/dev/CommonStyles/Common_themeresources_any.xaml"
"Title": "WinUI Theme Resources (GitHub)",
"Uri": "https://github.com/microsoft/microsoft-ui-xaml/blob/winui3/release/1.4-stable/controls/dev/CommonStyles/Common_themeresources_any.xaml"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm worried about this becoming quickly out of date with a new release. It will be easy to forget to update this link.

Can we dynamically create this uri string with SourcePath?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True, the bigger question I guess is if the WinUI 3 branch should be the default branch on the repo :)? So we can just link to main?

That would solve microsoft/microsoft-ui-xaml#9055 and microsoft/microsoft-ui-xaml#9052

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll start an email thread to discuss this internally.

Copy link
Collaborator

Choose a reason for hiding this comment

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

While that's being discussed, we should still avoid hard coding the branch name.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about just storing the in repo location e.g. controls/dev/CommonStyles/Common_themeresources_any.xaml and use the current version of the WinApp SDK being used by the WinUI Gallery to determine the branch name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup agreed.

FYI, it's already out of date in the stable version.
image

@chingucoding I think what makes this a bit more complex is that all other URLs are defined in the .JSON and are not dynamically generated in the UI? But please correct me if I'm wrong!

Copy link
Collaborator

Choose a reason for hiding this comment

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

It wouldn't be the first time we do this dynamically. The links to the source code for the sample pages is also dynamically generated based on the page name (and so far it worked quite well). The place to implement this should also be fairly straightforward, either do it while parsing the JSON or do it when rendering.

@@ -33,6 +33,9 @@ namespace AppUIBasics
/// </summary>
public sealed partial class ItemPage : Page
{
private static string WinUIBaseUrl = "https://github.com/microsoft/microsoft-ui-xaml/tree/winui3/release/1.4-stable/controls/dev/";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@karkarl @chingucoding Same here. Wish we could use a aka.ms/latestwinuipath instead and then append the SourcePath, but that does not seem to work :(

Copy link
Collaborator

Choose a reason for hiding this comment

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

The more I think about it, do we really want to link to the absolutly latest hot of the press release or the one people are seeing in the app? I would argue, if I see a control in the WinUI Gallery, I want to see the source code that made the control the way it is in the gallery and not the one inside the latest release. Otherwise, it might be a confusing experience when copying it over into one's own code and the control looking different then.

@karkarl karkarl merged commit 86b8b56 into main Feb 13, 2024
2 checks passed
@karkarl karkarl deleted the niels9001/sourcelinks branch February 13, 2024 00:01
karkarl pushed a commit that referenced this pull request Jul 15, 2024
## Description
This PR adds a direct link to the WinUI 3 control source code
(addressing: #1388):
- A path to a control folder or XAML file (when the control is shipped
with the OS) are defined in the `ControlDataInfo.json`
- On the item page, a link is shown (when available) in the Source
flyout. If not, the button is hidden.

This makes it easy for developers to quickly go to the source code /
default XAML file of a control.

When a path is configured in `ControlDataInfo.json`:

![image](https://github.com/microsoft/WinUI-Gallery/assets/9866362/87d7e0fa-edeb-468a-9e17-d8949e71cb6f)

the link navigates to:
https://github.com/microsoft/microsoft-ui-xaml/tree/winui3/release/1.4-stable/controls/dev//ParallaxView

If no link is configured, the button is hidden:

![image](https://github.com/microsoft/WinUI-Gallery/assets/9866362/3da75909-7a54-4ca4-a772-33e71544b7b4)



## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

---------

Co-authored-by: Jay <65828559+Jay-o-Way@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Suggestion] Add link to _themeresources.xaml for each control
4 participants