Remove default selection on console logs page#1119
Conversation
tlmii
requested review from
JamesNK,
adamint,
drewnoakes,
leslierichardson95 and
smitpatel
ghost
added
the
| <h1>Console Logs</h1> | ||
| <FluentToolbar Orientation="Orientation.Horizontal"> | ||
| <FluentSelect @ref="_resourceSelectComponent" TOption="ResourceViewModel" | ||
| <FluentSelect @ref="_resourceSelectComponent" TOption="Option<string>" |
There was a problem hiding this comment.
That'll allow us to add icons easily too if we decide to do so.
There was a problem hiding this comment.
How long has Option<T> been in FluentUI? I added our own version - SelectViewModel<T> - because I never noticed it.
There was a problem hiding this comment.
Hmm, why does Option<T> have Text and Value properties both of T? Surely Text should always be a string...
There was a problem hiding this comment.
How long has
Option<T>been in FluentUI? I added our own version -SelectViewModel<T>- because I never noticed it.
It's been there since August I guess. I only really noticed it when I looked into how to add icons and it looks like using Option<T> is the only way to do so:
Hmm, why does
Option<T>have Text and Value properties both ofT? Surely Text should always be a string...
Yeah I don't know, it seems off. Seems like Value should be T and Text should be string. But there's no special handling for Option<T> (other than Icon) so you still have to use the OptionText and OptionValue parameters to set the display text and value.
| private CancellationTokenSource? _watchLogsTokenSource; | ||
| private string _status = LogStatus.Initializing; | ||
|
|
||
| private readonly TaskCompletionSource _renderCompleted = new(); |
There was a problem hiding this comment.
Just append TaskCompletionSource or something. The name looks like a bool otherwise.
| private CancellationTokenSource? _watchLogsTokenSource; | ||
| private string _status = LogStatus.Initializing; | ||
|
|
||
| private readonly TaskCompletionSource _renderCompletedTaskCompletionSource = new(); |
There was a problem hiding this comment.
nit: The usual suffix (at least in aspnetcore code) is tcs
| private readonly TaskCompletionSource _renderCompletedTaskCompletionSource = new(); | |
| private readonly TaskCompletionSource _renderCompletedTcs = new(); |
There was a problem hiding this comment.
We are really inconsistent at this. e.g. _watchContainersTokenSource (now why it uses containers lol). 🙃
There was a problem hiding this comment.
TaskCompletionSource = tcs
CancelationTokenSource = cts
There was a problem hiding this comment.
Went ahead and renamed _watchContainersTokenSource too.
now why it uses containers lol
Naming things is hard to do. Renaming things is hard to remember to do.
* Remove default selection on console logs page * PR Feedback * More PR Feedback
3 participants
Resolves #1055
FluentSelectto be a list ofOption<string>instead of directly usingResourceViewModel(Select a resource)entry in the list (see first screenshot)/ConsoleLogsurl or using the menu, logs won't start automatically and you'll get the view in the second screenshot belowScreenshots:
