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

Blog template TS migration #121

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

Blog template TS migration #121

wants to merge 2 commits into from

Conversation

DevAOC
Copy link
Collaborator

@DevAOC DevAOC commented Nov 11, 2024

No description provided.

@DevAOC DevAOC requested a review from rdraward November 11, 2024 16:16
Copy link
Collaborator

@rdraward rdraward left a comment

Choose a reason for hiding this comment

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

This one looks good and works well. One suggestion for typing but otherwise good


// reformat the content field to be a markdown field
setValue("content", {
markdown: content,
});
} as any);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of doing this, we should be able to type the default value (line 40 of this file)

content: "Start writing your blog post here!" as string | { markdown: string | (string & { markdown: string | null | undefined; }); },

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll also try to find a big of time to clean this up before we pivot outside of Shopify apps, it is an icky MDXEditor integration right now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried that type but it didn't seem to work. Not sure what to do in this case. Do you have an suggestions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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