-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
feat: Remember Active Dashboard Chat User, Message Text, and Replying-To #2772
Conversation
- Caches the active chatter in the dashboard when the chat-messages controller gets reloaded. - This intentionally **does not** get saved to disk, and the application will always start back up with "Streamer" as the active chat sender. - Previously, changing component tabs then changing back would cause the chat sender to get reset to Streamer. This is *not* cool. - Implements crowbartools#2353
Nice one, I noticed this in my re-testing of changes for #2753 👍 |
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.
This is a much needed improvement, thanks for doing it!
One minor suggestion: since we just want to cache UI state in memory, I think it would make most sense to store the chatSender
variable in the associated chat-messages.service
instead of using the settings service. "Settings" in this context currently represents any user defined settings that we want to persist to file, so having a memory only setting here to store UI state feels outside the scope. In the event we do want to persist the selected chatter to file though, we could move to using settings!
General implementation:
// chat-messages.service.js
service.chatSender = "Streamer";
// chat-messages.controller.js
$scope.setChatSender = function(chatter) {
chatMessagesService.chatSender = chatter;
};
<!-- _chat-messages.html (chat messages service already passed to scope as $scope.cms) -->
<span class="ml-4" style="width: 100%; text-align: center">{{cms.chatSender}}</span>
Thanks for the nudge away from Should have this PR updated within a few hours. |
- Migrated chat/dashboard caching from settingsService to chatMessageService. - Added caching for chat sender (bot vs streamer). - Added caching for outgoing message text that has not (yet) been sent. - You can now select your chat sender, type out a beautiful message, change to another tab in the application for any reason, then come back and further edit or send the message.
- 3x "Attribute value missing" warns - "href" keys inside of "a" tags aren't mandatory - But an "href" key must have a value assigned - Removed unnecessary "href" keys. - 1x "Element 'div' cannot be nested inside of element 'span' - Self-explanatory - Changed outer span to a div.
- Add message reply/threading caching to the chat messages control. - Why remember 2/3rds but ignore the last 1/3rd? - I think I'm done touching this PR now, outside of potential v5 merges. - For real this time.
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.
Ooo great idea to update reply-to and message text as well. This will be so nice. Thanks for rockin' this!
Description of the Change
Applicable Issues
#2353
Testing
Yup, video evidence of functionality:
Screenshots
N/A