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

Refactor to allow for more flexibility and customization options in the future #45

Merged
merged 6 commits into from
May 4, 2024

Conversation

Adammatthiesen
Copy link
Member

@Adammatthiesen Adammatthiesen commented May 3, 2024

Working on topics discussed during the discord community call on May 1, 2024.

It adds a few useful tricks into the mix including moving the content render into a virtual component that is exported as well as setting up a proper d.ts file export using the AIK addDts() utility to allow the recognition of the new Virtual component Utilities "studiocms:components" as well as reorganizing the page routes as a start to creating a more flexible setup for the future!

@Adammatthiesen Adammatthiesen added enhancement New feature or request refactor labels May 3, 2024
@Adammatthiesen Adammatthiesen self-assigned this May 3, 2024
@Adammatthiesen Adammatthiesen requested a review from a team May 3, 2024 06:11
Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

LGTM ✅

image

@@ -12,9 +13,31 @@ type Props = {
title: string;
description: string;
heroImage?: string;
posts: []
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
posts: []
posts: BlogPost[]

Copy link
Member Author

@Adammatthiesen Adammatthiesen May 3, 2024

Choose a reason for hiding this comment

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

added in newest commit where i am now actually duplicating some of the astroDB runtime types for the DB tables. 🎉 and implemented your suggestion as well

… data used on multiple pages instead of just a string
Copy link
Contributor

@jdtjenkins jdtjenkins left a comment

Choose a reason for hiding this comment

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

Looks good!

@BryceRussell
Copy link
Member

These changes are great, they will make it a lot easier to migrate towards ATP. LGTM! 🚀

Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

Aside from the ts yelling everything looks good to me.

Comment on lines +146 to +153
//@ts-ignore
const htmlContent = await renderMarked(markdownContent.value);
//@ts-ignore
htmlPreview.innerHTML = htmlContent;
} else if (Config.contentRenderer === 'markdoc') {
//@ts-ignore
const htmlContent = await renderMarkDoc(markdownContent.value);
//@ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

what's the issue with ts here?

Comment on lines +140 to +147
//@ts-ignore
const htmlContent = await renderMarked(markdownContent.value);
//@ts-ignore
htmlPreview.innerHTML = htmlContent;
} else if (Config.contentRenderer === 'markdoc') {
//@ts-ignore
const htmlContent = await renderMarkDoc(markdownContent.value);
//@ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

same question here

Comment on lines +140 to +147
//@ts-ignore
const htmlContent = await renderMarked(markdownContent.value);
//@ts-ignore
htmlPreview.innerHTML = htmlContent;
} else if (Config.contentRenderer === 'markdoc') {
//@ts-ignore
const htmlContent = await renderMarkDoc(markdownContent.value);
//@ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

yep, here as well.

Comment on lines +148 to +155
//@ts-ignore
const htmlContent = await renderMarked(markdownContent.value);
//@ts-ignore
htmlPreview.innerHTML = htmlContent;
} else if (Config.contentRenderer === 'markdoc') {
//@ts-ignore
const htmlContent = await renderMarkDoc(markdownContent.value);
//@ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

here as well.

@Adammatthiesen
Copy link
Member Author

!coauthor

Copy link
Contributor

github-actions bot commented May 4, 2024

Co-authored-by: Bryce Russell <brycetrussell@gmail.com>
Co-authored-by: Jacob Jenkins <7649031+jdtjenkins@users.noreply.github.com>
Co-authored-by: Paul Valladares <85648028+dreyfus92@users.noreply.github.com>

@Adammatthiesen Adammatthiesen merged commit 6c19b55 into main May 4, 2024
1 check passed
@Adammatthiesen Adammatthiesen deleted the refactor/pages-authhandling branch May 4, 2024 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants