-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Update SignalR Sample #13078
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
Update SignalR Sample #13078
Conversation
@@ -83,6 +84,13 @@ <h4>Group Actions</h4> | |||
<script src="lib/signalr/signalr-protocol-msgpack.js"></script> | |||
<script src="utils.js"></script> | |||
<script> | |||
var name = prompt("Please enter your name", "D'Artagnan"); |
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.
Can we just have a normal text input box with a default value of "Anonymous" or "D'Artagnan" or whatever for the user name? That way it doesn't block the entire browser UI.
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.
Agreed. We had the prompt
in other places where we connected on page load. If we have a "Connect" gesture we can use a standard text box. I would also suggest having the "Connect" button check for a non-empty user name and produce an error (and not connect) in that case.
{ | ||
await Clients.Others.SendAsync("Send", $"{Context.ConnectionId} left"); |
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.
Can we include the user name in the query string or something so we can keep the user joined/left messages?
@@ -27,6 +27,7 @@ <h1 id="head1">SignalR Hubs Sample</h1> | |||
<option value="hubT">Hub<T></option> | |||
</select> | |||
|
|||
<input type="text" id="displayname" placeholder="Enter User Name" /> |
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.
I miss D'Artagnan
😄
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.
Same!
Making our sample look nicer. This is more similar to what we have in the samples repo.