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

Implement rudimentary redirect banner #155

Merged
merged 5 commits into from
Jan 3, 2025

Conversation

FlaminSarge
Copy link
Contributor

@FlaminSarge FlaminSarge commented Dec 23, 2024

Supports send and receive, uses the urlEndpoint or watchEndpoint link to open a new tab with the given url if the button is clicked
This also adds support for bold text (but not deemphasized text yet) to MessageRun
This does not implement contextMenuButton at all yet, only inlineActionButton.
Has the same auto-collapse behavior as other banner messages.

Built on top of #156

Very hacky URL handling and button handling at the moment.
"Redirect Notice" is added to the header because it looks a bit silly without anything there.
"Redirect profile photo" is set as the alt text for the image since the author's actual name is only available elsewhere and it would be too hacky to try to grab it.

Tested on mv3 w/ chrome

Screenshot 2024-12-23 at 3 11 25 AM Screenshot 2024-12-23 at 3 11 03 AM Screenshot 2024-12-25 at 6 54 14 PM

@FlaminSarge FlaminSarge force-pushed the redirect branch 3 times, most recently from eb1fa02 to e658b7c Compare December 25, 2024 23:02
Supports send and receive, uses the urlEndpoint or watchEndpoint link to open a new tab with the given url if the button is clicked
This also adds support for bold text (but not deemphasized text yet) to MessageRun
This does not implement contextMenuButton at all yet, only inlineActionButton.
Comment on lines +119 to +121
const url = baseRenderer.inlineActionButton?.buttonRenderer.command.urlEndpoint?.url ||
(baseRenderer.inlineActionButton?.buttonRenderer.command.watchEndpoint?.videoId ?
"/watch?v=" + baseRenderer.inlineActionButton?.buttonRenderer.command.watchEndpoint?.videoId
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This handling should probably be cleaned up at some point.

@KentoNishi
Copy link
Member

code looks good, but how should i actually test this without catching a redirect by sheer luck? how did u test it?

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Jan 1, 2025

code looks good, but how should i actually test this without catching a redirect by sheer luck? how did u test it?

This is fully tested, at this point.
I did manage to catch an outbound redirect by just leaving a stream open in Holodex (so it didn't auto-send to next page), as well as catching an inbound redirect in a similar fashion. The banner would auto-collapse, but would not go away since this impl leaves it there.

I also lucked out today in that 2 different people raided someone, and the new redirect banner overwrote the old one, so that's confirmed to work as well. I believe Svelte's reactive statements means that any time the 'redirect' object gets swapped out for a new redirect, it un-collapses and starts a new timer too.

Prior to that, during initial implementation/testing, I grabbed some actual data and mocked it in via one of these two in parseChatResponse:

  parsedResponse.continuationContents?.liveChatContinuation?.actions?.push(JSON.parse(mockData))
// ---
  actionsArray.push(JSON.parse(mockData))

Other considerations from stock YT behavior that could affect what order we show these in:
Outbound redirect lasts indefinitely.
Inbound redirect lasts 20s then disappears.
Chat summary ONLY appears when reloading live chat (never as a chatcontinuation), lasts 12s, then disappears.
Pinned message lasts 8s then collapses, only disappears when dismissed by the user or unpinned by streamer.

Since all of these only collapse instead of disappear under this Hyperchat impl, I'm not sure what order to put them in.

@KentoNishi
Copy link
Member

order probably isn't too big of a deal tbh

@KentoNishi
Copy link
Member

image

@KentoNishi
Copy link
Member

@FlaminSarge lmk if ur happy with the small tweaks i made, if it's all good with u ima merge

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Jan 2, 2025

@FlaminSarge lmk if ur happy with the small tweaks i made, if it's all good with u ima merge

Now that the button has moved:
Do we need to modify the text to match how Pinned messages put the author photo on the left and align the text so that it doesn't go into the photo's column? (same goes for realigning chat summary, but that doesn't have an image unless we use YT's little star icon) e.g.
image

Also the mock data doesn't really seem necessary to keep in. Trying to figure out if there's a good way to tell if YT's format changes, for which we should refuse to render anything for this to avoid breakage.

@KentoNishi
Copy link
Member

oh perchance that's a good idea, if u could fix that up that'd be great and ill merge

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Jan 3, 2025

oh perchance that's a good idea, if u could fix that up that'd be great and ill merge

how does this look? with/without the profile icons enabled

Something's off with the spacing above the text/image though, it doesn't quite match pinned
Updated, I feel like I'm just throwing random classes on there since I don't fully understand what they do though.

New:
Screenshot 2025-01-02 at 5 37 24 PM
Screenshot 2025-01-02 at 5 37 33 PM

Old:
Screenshot 2025-01-02 at 5 30 04 PM
Screenshot 2025-01-02 at 5 29 57 PM

@KentoNishi
Copy link
Member

checking over now

@KentoNishi
Copy link
Member

yep seems to work and looks good to me! tyty

@KentoNishi KentoNishi merged commit a0a88c2 into LiveTL:master Jan 3, 2025
@FlaminSarge FlaminSarge deleted the redirect branch January 3, 2025 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants