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

feat: Add support for Notion callout blocks #66

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kaspernowak
Copy link

@kaspernowak kaspernowak commented Nov 29, 2024

This PR adds support for Notion callout blocks with emoji-based styling, following ReadMe's markdown callout syntax.

Changes

  • Add callout block support with emoji-based color mapping
  • Implement ReadMe-style markdown callout syntax (https://docs.readme.com/rdmd/docs/callouts)
  • Add comprehensive documentation and examples
  • Add tests for callout parsing and conversion
  • Update type system for better type safety

Supported callout styles

  • 📘 Blue background (notes, information)
  • 👍 Green background (success, tips)
  • ❗ Red background (warnings, important)
  • 🚧 Yellow background (WIP, caution)

Example usage

> 📘 **Note:** This is a callout
> With multiple lines

@EndBug
Copy link
Member

EndBug commented Nov 29, 2024

This looks super cool!
My only concern is that this feature is a part of ReadMe markdown, not of GitHub-Flavored Markdown.
Acutally, GFM has a similar feature called alerts: https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#alerts

So I don't think this should be enabled by default, but it would be a cool feature to put behind some kind of flag!

@kaspernowak
Copy link
Author

kaspernowak commented Nov 29, 2024

GFM has a similar feature called alerts: https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#alerts

I could change it to use the GFM syntax by default, and add a flag that uses the "enhanced" ReadMe markdown version instead (or as well?), which would allow more callout "titles", icons (emojis) and colors (based on the emoji to color map).

What do you think @EndBug?

@kaspernowak
Copy link
Author

@EndBug I added the GFM syntax now as the default callout syntax. The ReadMe markdown syntax (emoji callouts) now need to be enabled by setting:

const options = {
  enableEmojiCallouts: true,
};

Would this work for you?

@EndBug EndBug force-pushed the feature/add-callout-block-support branch from 2e27970 to 496fee9 Compare January 2, 2025 11:10
@kaspernowak
Copy link
Author

@EndBug Thanks for improving the implementation!

@kaspernowak
Copy link
Author

kaspernowak commented Jan 2, 2025

@EndBug I just took a closer look, and I am curious as to why you removed the children parameter from the exported callout block function?

In src/parser/internal.ts, the children that need to be added to the callout block based on the markdown, are defined for both callout types in the parseBlockquote function:

function parseBlockquote(
  element: md.Blockquote,
  options: BlocksOptions
): notion.Block {
  const firstChild = element.children[0];
  const firstTextNode =
    firstChild?.type === 'paragraph'
      ? (firstChild as md.Paragraph).children[0]
      : null;

  if (firstTextNode?.type === 'text') {
    // Helper to parse subsequent blocks
    const parseSubsequentBlocks = () =>
      element.children.length > 1
        ? element.children.slice(1).flatMap(child => parseNode(child, options))
        : [];

    // Check for GFM alert syntax first (both escaped and unescaped)
    const firstLine = firstTextNode.value.split('\n')[0];
    const gfmMatch = firstLine.match(
      /^(?:\\\[|\[)!(NOTE|TIP|IMPORTANT|WARNING|CAUTION)\]$/
    );

    if (gfmMatch && notion.isGfmAlertType(gfmMatch[1])) {
      const alertType = gfmMatch[1];
      const alertConfig = notion.GFM_ALERT_MAP[alertType];
      const displayType =
        alertType.charAt(0).toUpperCase() + alertType.slice(1).toLowerCase();

      const children = [];
      const contentLines = firstTextNode.value.split('\n').slice(1);

      if (contentLines.length > 0) {
        children.push(
          notion.paragraph(
            parseInline({
              type: 'text',
              value: contentLines.join('\n'),
            })
          )
        );
      }

      children.push(...parseSubsequentBlocks());

      return notion.callout(
        [notion.richText(displayType)],
        alertConfig.emoji,
        alertConfig.color,
        children
      );
    }

    // Check for emoji syntax if enabled
    if (options.enableEmojiCallouts) {
      const emojiData = notion.parseCalloutEmoji(firstTextNode.value);
      if (emojiData) {
        const paragraph = firstChild as md.Paragraph;
        const textWithoutEmoji = firstTextNode.value
          .slice(emojiData.emoji.length)
          .trimStart();

        // Process inline content from first paragraph
        const richText = paragraph.children.flatMap(child =>
          child === firstTextNode
            ? textWithoutEmoji
              ? parseInline({type: 'text', value: textWithoutEmoji})
              : []
            : parseInline(child)
        );

        return notion.callout(
          richText,
          emojiData.emoji,
          emojiData.color,
          parseSubsequentBlocks()
        );
      }
    }
  }

  const children = element.children.flatMap(child => parseNode(child, options));
  return notion.blockquote([], children);
}

I added the children parameter, as I assumed the callout block function should be defined in the same way or similar to the blockquote block function. By removing the children parameter from the callout block, we break this functionality.

Copy link
Member

@EndBug EndBug left a comment

Choose a reason for hiding this comment

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

Heyy
So I did push a commit, but I then removed it because I saw it didn't match Notion's API.
I now fixed a couple of issues, but I think the main one is that the Notion client library is not up to date

We shuould try updating that to see if they have better typings

@kaspernowak
Copy link
Author

kaspernowak commented Jan 3, 2025

@EndBug I have a clean branch from master where I have updated the Notion client library here:
https://github.com/kaspernowak/martian/tree/update/notion-client-2.2.15

I will make it into a PR, then if we can merge that successfully, we can use that to finalise this implementation.

@kaspernowak
Copy link
Author

kaspernowak commented Jan 3, 2025

@EndBug I have added the PR here

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