-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add unhighlight on disconnect #67
Conversation
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.
Working as advertised and good to go as it is.
From a user-friendliness perspective I might suggest a slightly different approach. The discoverability of this is not particularly kind; have a third option for "prompt when disconnecting" and enable it by default with an option in the popup to not make a choice and not ask again?
Other concerns, it's possible to disconnect unintentionally, for instance a service blip from twitch API or whatever, and losing all your highlights when you could easily reconnect is pretty crappy. If it's possible to identify when a disconnect happened for some reason other than user intentionally disconnecting, then prompt regardless, that would be a nice to have.
The user will be presented with a dialog that will ask what to do with the highlights when disconnecting from Twitch. 1) Always Remove - Sets the `unhighlightOnDisconnect` setting to true (will not display dialog again) and unhighlights all. 2) Remove - Unhighlights all. 3) Keep - Does nothing with the highlights.
I decided to keep the
|
@NickLarsen I tested locally and believe @parithon has addressed your valid concerns around the user-friendliness of this feature. When you get a moment could you re-review and let us know what you think. Essentially now there is a notification to prompt you on how you want to handle the highlights on disconnect. Also if there is an interruption in the connection to the Twitch API that will not affect the highlights. The server implementation is set to attempt to reconnect as well. |
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'm going to approve for now so we can release and let people test that way too. If there is further feedback on this that would require some changes we can cross that bridge then. Thanks everyone for the help and input!
Sorry for not getting back to this earlier. From the screenshot it looks like an improvement and keeping it off by default is fine for now. |
Purpose
When streamers disconnect from the chat service it would be helpful if the highlights that were generated while on stream automatically go unhighlight.
Files Changed
extension.ts
: added the ability to automatically unhighlight when disconnecting from the chat service when the user sets theunhighlightOnDisconnect
setting.package.json
: added theunhighlightOnDisconnect
setting to the configuration section.How to Test
git clone https://github.com/clarkio/vscode-twitch-highlighter.git
cd vscode-twitch-highlighter && npm install
Twitch Highlighter Unhighlight On Disconnect
in your workspace settingsWhat to Validate
Addresses
#64