Skip to content

Conversation

@knowler
Copy link
Contributor

@knowler knowler commented Dec 17, 2022

Usually I like to keep PRs scoped smaller, but I had some ideas to enhance/improve the initial implementation. Not all the changes herein are intentional.

First off, I thought that we could support both types of toot URLs (see code for examples). To do this, I am using the URL Pattern API which is only supported in Chromium. We could always use regex to do this. I’m getting the toot ID to just make API url for it. You’ll notice I was lazy with my TypeScript here.

Second, I thought we could use ElementInternals to improve the accessibility. I didn’t check what Tweet embeds do which might have been a better guide. Instead, I used a Tweet/Toot from the timeline as my reference point. Both use the <article> element and corresponding role and then create the label from selected text content inside of the post.

Third, I did move the setting of the ShadowRoot.innerHTML to the #render method, since I just found it nicer when adding all of the content. We could always move it back.

Finally, there’s an initial flash of unpleasant styling when none of the content exists. I used a CustomStateSet pseudo-selector (i.e. --loading) to avoid this. We could always repurpose this for an actual loading state.

Anyway, I thought I’d make a draft out of this. I could always break this into smaller PRs as well. I made a pen where I was experimenting with some styling: https://codepen.io/knowler/pen/oNMvYwm. I only made progress on the dark mode styling. The pen has slightly different code than this PR.

@knowler
Copy link
Contributor Author

knowler commented Dec 17, 2022

Oooo, and we could make use of <spoiler-text> too!

Copy link
Member

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

These changes look great! Let's add them in 😄

@keithamus keithamus marked this pull request as ready for review January 3, 2023 15:19
@keithamus keithamus merged commit 4b58cb5 into 24webcomponents:main Jan 3, 2023
@knowler knowler deleted the ideas-for-improvements branch January 4, 2023 03:56
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