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

new plugin YoutubeDescription #2427

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

new plugin YoutubeDescription #2427

wants to merge 2 commits into from

Conversation

arHSM
Copy link
Contributor

@arHSM arHSM commented May 9, 2024

Adds descriptions to youtube video embeds

demo

@bignutty
Copy link

bignutty commented May 9, 2024

LGTM

@Cynosphere
Copy link
Contributor

I would personally prefer having it unexpanded by default, showing the first line, and then being able to click to show the rest.

@Golonchy
Copy link

W

@arHSM
Copy link
Contributor Author

arHSM commented May 10, 2024

@Cynosphere added

@arHSM
Copy link
Contributor Author

arHSM commented May 10, 2024

out

return !isNonNullish(embed.rawDescription)
? null
: embed.rawDescription.length > 20
? <div
Copy link
Owner

Choose a reason for hiding this comment

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

this nested ternary isn't too great.. it'd be way nicer for readability to convert it into simple if statements, like

if (!embed.rawDescription)
	return null;
if (embed.rawDescription.length <= 20)
	return original();

return (
	<div>
		....

}
}
],
ToggleableDescription({ embed, original }: ToggleableDescriptionProps) {
Copy link
Owner

Choose a reason for hiding this comment

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

this should be wrapped in an ErrorBoundary, then you also wouldn't need your Wrapper function

Suggested change
ToggleableDescription({ embed, original }: ToggleableDescriptionProps) {
ToggleableDescription: ErrorBoundary.wrap(({ embed, original }: ToggleableDescriptionProps) => {

ToggleableDescription({ embed, original }: ToggleableDescriptionProps) {
const [isOpen, setOpen] = useState(false);

console.warn(embed);
Copy link
Owner

Choose a reason for hiding this comment

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

please remove this console.log

Suggested change
console.warn(embed);

@Vendicated
Copy link
Owner

maybe an improvement to the collapsing could be to collapse on the first newline instead of just after some characters? with a sanity limit of like 200 characters or something

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.

None yet

5 participants