Skip to content

Conversation

@DeSoRSS
Copy link

@DeSoRSS DeSoRSS commented Aug 7, 2024

Added Twitter Embed functionality
e.g. https://twitter.com/teslaownersSV/status/1819749725568110806
or https://x.com/teslaownersSV/status/1819749725568110806

Twitter Embed IFRAME has been given 500px height - this accommodates shorter tweets without requiring the user to scroll the iframe...but longer tweets will still require internal iframe scrolling. This 500px seemed like a good height trade-off i.e. choosing between: avoiding scrolling in "most cases" vs introducing a lot of extra white-space when the tweet is short but the IFRAME was hardcoded to something like 700px. ...setting a fixed IFRAME height will never be perfect when the inner content height is unknown.

Please note that Twitter's Tweet.html page contains a HTML tag with style for overflow-y: scroll. Which forces a scrollbar to be visible inside the IFRAME (even if the content height is less than the IFRAME height). This is unavoidable.
<html dir="ltr" lang="en" style="overflow-y: scroll; overscroll-behavior-y: none;">

I've also gone ahead and fixed the TikTok Embed height, previously it was given 700px height when posting, but then 315px height when viewing a post (a bug)...the cause of this was isValidTiktokEmbedURL did not match the EmbedVideoURL. So I added an additional check against isTikTokLink to accommodate for both URLs (normal url and embed url). I also updated to 760px height for TikTok embeds as this height more reliably avoids seeing a scrollbar inside the IFRAME.

@DeSoRSS
Copy link
Author

DeSoRSS commented Aug 7, 2024

Video evidence of the Twitter Embed (new) and TikTok Embed (fix):
https://diamondapp.com/posts/17d3f7017ed36c52a10c3fcb46475407bcef6aaa5273a624f0ebfc1616d5ec4d

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.

1 participant