Code Quality: Introduced IWindowsAppLauncherService #15927
Conversation
0x5bfa
force-pushed
the
5bfa/CQ-IWindowsLaunchService
branch
from
ac71456
to
42a305b
Compare
0x5bfa
changed the title
|
I have CreateFile method using Vanara in this file yet as well. Also, merging this after v3.6 looks good to me. |
There was a problem hiding this comment.
Could you please add some comments? I understand this is based on existing code, but our standard practice is to include comments when creating an interface. Thanks!
There was a problem hiding this comment.
Ah I forgot that even though I appended inheritdoc to implementations lol
| @@ -52,7 +53,7 @@ private async void GoToStorageSense_Click(object sender, RoutedEventArgs e) | |||
| button.Tag.ToString() is not string path) | |||
| return; | |||
|
|
|||
| await StorageSenseHelper.OpenStorageSenseAsync(path); | |||
| await WindowsLaunchAppService.LaunchStorageSensePolicySettingsAsync(); | |||
There was a problem hiding this comment.
Should you include the path in the arguments?
There was a problem hiding this comment.
The first implementation to include path is made by Gave (3 yrs ago) but I'm not sure why because Storage Sense works for only system drive and for the whole system drive not for specifically a path.
There was a problem hiding this comment.
Actually path isn't used to be passed to settings to give extra context.
There was a problem hiding this comment.
There is a thread on GitHub about this with more details, but in any event, it makes a difference for which page is opened in Settings.
There was a problem hiding this comment.
Both open wrong page.
For other than C (system) drive:
For thers:
Both should open:
There was a problem hiding this comment.
My implementapention opens what should be opened.
There was a problem hiding this comment.
Both open wrong page.
It's supposed to open the Storage Page for the C:\ drive, and the "Storage used on other drives" page for other drives.
There was a problem hiding this comment.
Even so, how would you free up storage for other drives? The "Storage used on other drives" doesn't contains any free up page.
As to C, why not open Policy page directly.
There was a problem hiding this comment.
I assume this weird page navigation might come from win10 when Gave implemented
There was a problem hiding this comment.
Even so, how would you free up storage for other drives? The "Storage used on other drives" doesn't contains any free up page.
You can access the "Free up storage" page by clicking on a drive. Ideally we'd open directly to that page, but that's not supported by the Settings app.
|
This should not be DI'd. |
Resolved / Related Issues
This also involves with IShellItem, methods of which return HRESULT but CsWin32 generates those methods with void returned. But this issue can be detoured by not allowing marshaling by CsWin32. However, this is blocked by WinUIEx because of CsWin32 issue, conflicting generated code modifiers between internal (WinUIEx) and public (Files). WinUIEx removal is almost done so this should be able to be continued soon again.
Steps used to test these changes
None yet