Conversation
| ArgumentException.ThrowIfNullOrWhiteSpace(displayName); | ||
|
|
||
| CommandType = commandType; | ||
| DisplayName = displayName; |
There was a problem hiding this comment.
Assuming we would want to localize the displayName?
There was a problem hiding this comment.
I assume we will want that localized somehow, along with things like custom statuses and whatnot. But I'm not sure what the plan is for that. Will discuss with folks, but I think that can come in a separate PR.
There was a problem hiding this comment.
We should consider sending user culture to hosting and letting hosting localize
|
Very cool @tlmii. I'm planning to look at implementing restart in Aspire.Hosting. The change looks like I would expect! We should think about future operations that may need more than a confirmation and make sure the resource server contract can expand to it. |
Nice - let me know what you think of the experience when you start that - I'm expecting that'll generate more feedback since it'll widen the audience for this UX.
Yeah I think this area will need more attention as we get a better idea of the commands that need to run through it. Even now with the confirmation dialog, it feels a bit forced. For example, what should the buttons say? Yes/No? OK/Cancel? Whatever we pick forces the language that can be used in the confirmation dialog. So even that could be more flexible - though we probably don't want to go the route of implementing an entire MessageBox API in the resource server api. |
41240dc to
30e7655
Compare
|
I went ahead and removed the test code. If anyone wants to run it, they can grab the 2nd to last commit (30e7655 as of this writing) to make testing easier - or just use the code in the original commit (c1a1262#diff-53cce2ea4b16776f87f7628686480c626c9fce25ea5a28eb46dd95f1dc72717f and c1a1262#diff-d35f6c3b9acebacf4a61b2055f987e92d1a207f889ac9892a6c4ad92f6492186) as a template for your own. |
|
I ran TestShop on the branch and didn't see the column. I'm guessing that means there are no commands. How do we test this? |
|
@JamesNK see my previous comment - you can pull the commit prior to the latest to get the test code I used or just use the small amount of test code from the first commit in the PR manually (the two files in Aspire.Hosting that have changes). |
|
Lets get this merged. I need to design the app model API for exposing this 😄 |
JamesNK
left a comment
There was a problem hiding this comment.
Feedback:
- The
...button disable state is very subtle. It needs to be more obvious. Instead of changing the opacity, maybe the color should be grey. - The menu goes off the bottom of the screen:

Fixed an issue like this in metrics. Does the control you're using have the same setting? #2301
Comments + issues here should be fixed but I trust you to do it in a follow up. Can merge now to unblock people
src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor
Outdated
Show resolved
Hide resolved
src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs
Outdated
Show resolved
Hide resolved
|
Thanks @JamesNK. I fixed a few of the things real quick before merge. Will tackle the rest in the morning in a separate PR. |
|
Need to rebase. |
Co-authored-by: James Newton-King <james@newtonking.com>
e62b439 to
394a91d
Compare
Yeah, I'm not real fond of it but it's how the built-in control appears to work. I wanted to look at that specifically to see if there was something built-in to handle it or if we needed to go custom with it. |
Filed #2661 to track microsoft/fluentui-blazor#1640 to address this. |
Figured it out - PR here: #2670 |

First pieces of the resource command support:
Here's a quick recording of it in action:
Some notes:
Microsoft Reviewers: Open in CodeFlow