-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[Settings > PT Run] Plugin manager: Add plugin version and website #36580
base: main
Are you sure you want to change the base?
Conversation
src/settings-ui/Settings.UI/SettingsXAML/Views/PowerLauncherPage.xaml
Outdated
Show resolved
Hide resolved
@davidegiacometti |
Nice! |
@htcfreek nice work! 😃 Yeah, I think we should use file version since .NET 8 introduced a breaking change on how revision is handled in product version: https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/source-link |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@davidegiacometti |
src/settings-ui/Settings.UI/SettingsXAML/Controls/MetadataDotSeparator.xaml
Outdated
Show resolved
Hide resolved
src/settings-ui/Settings.UI/ViewModels/PluginMetadataViewModel.cs
Outdated
Show resolved
Hide resolved
src/settings-ui/Settings.UI/ViewModels/PowerLauncherPluginViewModel.cs
Outdated
Show resolved
Hide resolved
src/settings-ui/Settings.UI/ViewModels/PluginMetadataViewModel.cs
Outdated
Show resolved
Hide resolved
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.
Hi @htcfreek
I have tested it and it's working well! I left some comment, let me know what you think.
I haven't noticed any performance drop compared to 0.87 but I am on a new hardware and this could make a difference.
Personal preference and for consistency with the OS I will replace the dot separator with a vertical one.
@davidegiacometti
|
I am fine with both. Just wanted to mention it because I like consistency with the OS. |
@niels9001, @davidegiacometti
I will try to do the changes within the next days. |
src/settings-ui/Settings.UI/SettingsXAML/Views/PowerLauncherPage.xaml
Outdated
Show resolved
Hide resolved
@niels9001 , @davidegiacometti I also tried the |
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 addressing the feedback. Looking so much cleaner!
I have tested it again and it's working well, I also didn't experience any performance regression.
Approved! ✅
@niels9001 |
<StackPanel | ||
HorizontalAlignment="Right" | ||
Orientation="Horizontal" | ||
Spacing="5"> |
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.
Spacing="5"> | |
Spacing="4"> |
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.
Any specific reason for this change? For me 4 feels to less and 6 feels to much. I like to keep 5.
(With 4 it looks more like a text character than a separator.)
Summary of the Pull Request
This PR implements a website link and the plugin version into the plugin metadata line visible in PT Run settings.
If the website uri in the
plugin.json
is badly formatted the link is not available in settings ui. This because we use a fallback uri in this case is to prevent a page crash.Sample screenshot with a third-party plugin
PR Checklist
Detailed Description of the Pull Request / Additional comments
Open tasks
Improve the XAML code for the items separating dot. (Optional.)Add template selector to switch between layout with and without website (In case the website uri is not well formed.)Validation Steps Performed
Manual tested with default plugins and different website uri strings.