Skip to content
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

Merged
merged 9 commits into from
Sep 13, 2024

Conversation

phroggster
Copy link
Collaborator

@phroggster phroggster commented Aug 31, 2024

Description of the Change

  • Caches the active chat sender, outgoing chat message text, and reply/thread information 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.
  • Does not add any actual settings, checkboxes, nor UI. It just remembers the thing you already changed while the application stays active.
  • Previously, changing component tabs then changing back would cause the chat sender to get reset to Streamer, the message text to be blank, and the reply-to/thread information to be dropped. This was not cool.

Applicable Issues

#2353

Testing

Yup, video evidence of functionality:
Video Of Tests

Screenshots

N/A

- 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
@phroggster phroggster added the Enhancement A feature request or improvement label Aug 31, 2024
@Wrongtown
Copy link

Nice one, I noticed this in my re-testing of changes for #2753 👍

Copy link
Member

@ebiggz ebiggz left a 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>

@phroggster
Copy link
Collaborator Author

Thanks for the nudge away from settingsService, that did seem like the wrong place for this. I'm going to take your wisdom into account, but also add caching for any pending message text. The breadth of this PR has now been bumped up a notch, ever since I typed out a wordy message then switched tabs to look up an identifier and lost my message text the other day. 😭

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.
@phroggster phroggster changed the title feat: Remember Active Dashboard Chat User feat: Remember Active Dashboard Chat User, Message Text, and Replying-To Sep 12, 2024
Copy link
Member

@ebiggz ebiggz left a 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!

@ebiggz ebiggz enabled auto-merge (squash) September 13, 2024 02:51
@ebiggz ebiggz merged commit 295d613 into crowbartools:v5 Sep 13, 2024
1 check passed
@phroggster phroggster deleted the feat_2353 branch September 13, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A feature request or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants