Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

We never actually made MoreCommands observable #360

Closed
zadjii opened this issue Jan 23, 2025 · 0 comments · Fixed by #433
Closed

We never actually made MoreCommands observable #360

zadjii opened this issue Jan 23, 2025 · 0 comments · Fixed by #433
Labels
Area-HostUX UX elements of the host CmdPal application Issue-Bug Something isn't working

Comments

@zadjii
Copy link
Collaborator

zadjii commented Jan 23, 2025

So, apps can't actually change the contents of the context menu at runtime. I mean they can, we just ignore them. There's a big TODO in CommandItemViewModel about it

@zadjii zadjii added Area-HostUX UX elements of the host CmdPal application Issue-Bug Something isn't working labels Jan 23, 2025
zadjii-msft added a commit that referenced this issue Jan 24, 2025
_targets #355_, which I need for improvements to messages

* [x] The initial package load takes a long time. This is pretty much unavoidable, but we do it on cmdpal startup, so anything after about 12s should be snappy
* [x] I cannot for the life of me get `FindPackagesAsync`, to be async. The call always ends up running synchronously, so I can't hook up the `operation.Completed` event, nor the cancellation. The action is already complete! 
  - this is probably blocking, because we still end up doing a search on most keystrokes, so we only get the final results after all the intermediate ones are done. 
  - Just pasting a search though? Just as snappy as you'd hope. 
  - Ahahahaha it wasn't me: [microsoft/winget-cli#5151](microsoft/winget-cli#5151)
  - ✅ manually wrapping this in a BG thread made it better
* [ ] We probably shouldn't make the default action for an installed package "Uninstall". 
  - Probably want to shunt over to the Settings app for the package
  - We probably want to do the thing where the second command doesn't show up if it's a separator
  - Punt? punt
* [x] We need to add more metadata in the details for packages. We have it, if only we could show it: #95 
  - This will be a follow-up
* [ ] This needs localization too
* I'm using the `1.10-preview` of the winget com interfaces. On my framework laptop at least, the `RefreshPackageCatalogAsync` API isn't yet implemented, so I need to test that
* [x] I don't think we implemented `MoreCommands` being observable in the host yet. We should.
  - Punted, #360 
* [ ] I probably also need to check if other APIs we're using exist or not
* [x] I haven't tested situations that like, need you to accept a license? Installing `nano` and the NanoLeaf app both _just work_. 
  - Punted?
zadjii-msft added a commit that referenced this issue Feb 19, 2025
I was testing #152 and realized that the Copy command on the calculator page was never working quite right.

And to make it work, i had to fix #360 

so, closes #360
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area-HostUX UX elements of the host CmdPal application Issue-Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant