-
Notifications
You must be signed in to change notification settings - Fork 64
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: Trubbel’s Utilities 2.4.1 #267
base: master
Are you sure you want to change the base?
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.
Couple minor things I'd like changed. The main ones being the duplication of methods available in site.fine
and the use of a hard-coded chat type. Thanks!
checkNavigation() { | ||
if (!this.settings.get("addon.trubbel.chat.moderation-bttv")) return; | ||
|
||
const chatRoutes = [ |
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.
When using the main flavor, site.constructor.CHAT_ROUTES
has a list of routes where chat features are expected to be enabled. It doesn't exist on clips flavor. That would be more accurate than maintaining your own list, though I don't consider this a blocker.
|
||
onRightClick(event) { | ||
// Helper function to get React's internal instance | ||
const getReactInstance = (element) => { |
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.
Please use the method provided via the site.fine
module to do this. The key is stable within any given page load and FFZ caches it. Likewise, there's methods for searching up and down the react tree.
} | ||
|
||
createCustomTimeout() { | ||
const container = createElement("div", { |
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.
You'd probably be saving yourself some annoyance if you just use the .jsx
extension for your files and write your DOM node creation code using JSX. FFZ's createElement
helper is compatible with the JSX transpiler used in the add-ons repo. There should be other add-ons that provide an example on how to do this.
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 actually tried doing that, but couldn't for the life of me figure out why things kept breaking with the SVG elements. Attributes were missing no matter what I did.
And I've tried once again, but brain isn't braining.
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.
That'd be an FFZ "bug". There's a list of attribute names in the DOM module that createElement uses for determining what values should be assigned with setAttribute and which ones should just be set on the JS object and I haven't put any svg attributes in the list. I'm doing that in today's update so creating SVG elements should work in the future.
if (!this.settings.get("addon.trubbel.chat.viewer-card")) return; | ||
|
||
const messageType = event?.message?.type; | ||
if (messageType !== 29) return; |
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.
You'll want to use the chat_types
enum available from the site.chat
module for this, rather than a hard-coded number. The message type IDs are not always stable, but the chat_types enum is loaded from Twitch's bundle so it stays up to date. In this case I believe you'd want chat_types.Info
checkNavigation() { | ||
if (!this.settings.get("addon.trubbel.chat.viewer-card")) return; | ||
|
||
const chatRoutes = [ |
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 thing as before, there's an existing list of chat routes you can use.
Added: