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

CTC-2289 add resolver section to RT docs #80

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

pokornyd
Copy link
Contributor

@pokornyd pokornyd commented Feb 5, 2024

@pokornyd
Copy link
Contributor Author

looking at this again and thinking... I wanted to have an extensive example, showcasing all the possibilities, but I guess it can be trimmed down slightly to not be so huge. let me know what you think.

Copy link
Contributor

@davidk2-kentico davidk2-kentico left a comment

Choose a reason for hiding this comment

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

1/ I wonder if the case inconsistences have technical reason. E.g., const MyComponent vs. const parsedTree.
2/ I think it'd be beneficial to name the link mark (or whatever's the terminology for this) in a more descriptive way, e.g. externalLink as opposed to internalLink you have there.
But those are minor things. In your code I trust. xD

@davidk2-kentico davidk2-kentico merged commit 5baa7e1 into Kontent-ai-Learn:main Mar 1, 2024
1 check passed
@pokornyd
Copy link
Contributor Author

pokornyd commented Mar 4, 2024

hey, thx for the review. let me follow up on your remarks :)

  1. this is actually in line with naming conventions, as MyComponent is a React component, whose name should be PascalCased, whereas parsedTree is a regular TS variable (or a constant to be precise)
  2. link is a default block type for external URLs in the PortableText standard, so I decided to keep it that way. internalLink is a custom block, representing a link to a content item, so it's Kontent.ai specific :)

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