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

add theme color ogp tag #235

Merged
merged 1 commit into from
Jan 24, 2022
Merged

add theme color ogp tag #235

merged 1 commit into from
Jan 24, 2022

Conversation

bnidevs
Copy link
Contributor

@bnidevs bnidevs commented Jan 24, 2022

No description provided.

Copy link
Member

@vcfxb vcfxb left a comment

Choose a reason for hiding this comment

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

This looks good, but does not fit with our current dynamic OGP tag system. Do we want theme color to be specifically static, or at least until it gets refactorred to be dynamic?

@bnidevs
Copy link
Contributor Author

bnidevs commented Jan 24, 2022

do .hbs files not allow for a combination of static and dynamic content?

@vcfxb
Copy link
Member

vcfxb commented Jan 24, 2022

No they do, I'm just saying that this breaks the pattern established by the other ogp tags being dynamic, and there should be a comment/justification on why that's the case with this one.

@bnidevs
Copy link
Contributor Author

bnidevs commented Jan 24, 2022

This meta tag strictly modifies the color theme of the blocks that are displayed on Discord/elsewhere, and I assumed it could be consistent across all usages of Telescope, unless you want varying colors based on link type?

Screen Shot 2022-01-23 at 8 49 27 PM

@vcfxb
Copy link
Member

vcfxb commented Jan 24, 2022

It might be nice to make any telescope link that errors out red, in theory. Although, after further consideration, that would be a bunch of work and a single color across-the-board is probably fine for now. This gets my approval

@vcfxb vcfxb merged commit 5dad382 into master Jan 24, 2022
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