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 all blocks to the readme table #63

Merged
merged 3 commits into from
Aug 25, 2022
Merged

Add all blocks to the readme table #63

merged 3 commits into from
Aug 25, 2022

Conversation

sHermanGriffiths
Copy link
Contributor

No description provided.

README.md Outdated
@@ -64,11 +64,47 @@ At the core of n2y are a set of python classes that represent the various parts

| Notion Object Type | Description |
| --- | --- |
| Block | A bit of content within a Page |
Copy link
Contributor

Choose a reason for hiding this comment

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

@sHermanGriffiths this table, which includes the Notion Object Types, should not include all of the individual block types. There is a table further down in the readme for the block types that contains some of the block types already.

Also, the descriptions should be empty unless there is something non-obvious about the behavior. Thus, comments like "A notion block of an image" should be omitted. This was the intent of the GitHub issue that stated "Only add information that is not obvious; e.g., if an paragraph block just makes a paragraph in markdown, we don't need to add any comment about it.". I think this will make the table more useful if it only contains the non-obvious stuff.

Copy link
Contributor

Choose a reason for hiding this comment

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

README.md Outdated
| VideoBlock | A block that houses a video |
| PdfBlock | A block that houses a PDF |
| ChildrenPassThroughBlock | A block that passes along the children of it's parent |
| ColumnBlock | A block that apears on the page as a column |
Copy link
Contributor

Choose a reason for hiding this comment

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

The description of the ColumnBlock should explain how we convert the column blocks into markdown. In particular, we ignore the columns and just display the content in each column one after the other in the markdown, since there is no way to support columns in markdown. This is a good example of the non-obvious sort of content that should go into this table.

Copy link
Contributor

@johndgiese johndgiese left a comment

Choose a reason for hiding this comment

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

@sHermanGriffiths I think there was some confusion regarding the tasks in #25

Please read the issue more carefully next time, as I believe some of the instructions you didn't follow seem pretty clear.

README.md Outdated
@@ -99,27 +99,36 @@ Here are the default block classes that can be extended:

| Class Name | Noteworthy Behavior |
| --- | --- |
| BookmarkBlock | |
| BookmarkBlock | Converts visual bookmark into plain text link in markdown, using the caption as the link text. |
| BreadcrumbBlock | Ignore |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| BreadcrumbBlock | Ignore |
| BreadcrumbBlock | These blocks are ignored |

README.md Outdated
| ColumnBlock | |
| ColumnListBlock | Columns are flattened and displayed sequentially. |
| ToggleBlock | Convert toggles into a details and summary tag. |
| VideoBlock | Acts the same way ass the Image block |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| VideoBlock | Acts the same way ass the Image block |
| VideoBlock | Acts the same way as the Image block |

README.md Outdated
| ToggleBlock | Convert the toggles into a bulleted list. |
| ColumnBlock | |
| ColumnListBlock | Columns are flattened and displayed sequentially. |
| ToggleBlock | Convert toggles into a details and summary tag. |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the current default functionality for the ToggleBlock is appropriate. Users can use a plugin if they want to produced details and summary tags. Keep in mind we are often exporting to non-HTML formats, like PDF or Word documents.

README.md Outdated
| SyncedBlock | Transcribe the contents of the synced block at the time it was constructed |
| TableBlock | |
| TableOfContentsBlock | transcribes the contenets of the Table of contents block into page links |
| TemplateBlock | Ignore |
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

README.md Outdated
| RowBlock | |
| SyncedBlock | Transcribe the contents of the synced block at the time it was constructed |
| TableBlock | |
| TableOfContentsBlock | transcribes the contenets of the Table of contents block into page links |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can ignore the table of contents blocks for now.

README.md Outdated
| NumberedListItemBlock | |
| ParagraphBlock | |
| PdfBlock | Acts the same way ass the Image block |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| PdfBlock | Acts the same way ass the Image block |
| PdfBlock | Acts the same way as the Image block |

| HeadingOneBlock | |
| HeadingTwoBlock | |
| HeadingThreeBlock | |
| ImageBlock | It uses the URL for external images, but downloads uploaded images to the `MEDIA_ROOT` and replaces the path with a relative url based off of `MEDIA_URL`. The "caption" is used for the alt text. |
| FileBlock | Acts the same way as the ImageBlock, except that in the documents it only ever shows the URL. |
| LinkToPageBlock | Transcribes the block into a plain text link |
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

README.md Outdated
| BulletedListItemBlock | |
| CalloutBlock | The content of the callout block is extracted, but the emoji and background color are ignored. |
| ChildPageBlock | |
| FencedCodeBlock | |
| ChildDatabaseBlock | Converts database to markdown in different file in directory and writes a relative link to the markdown|
Copy link
Contributor

Choose a reason for hiding this comment

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

The difficulty with exporting the child database to markdown is that there's no way to represent a database as a single markdown file. Depending on the selected output format, a database is either a single YAML file or many markdown files.

For now, I propose we just ignore child databases.

README.md Outdated
| ChildPageBlock | |
| FencedCodeBlock | |
| ChildDatabaseBlock | Converts database to markdown in different file in directory and writes a relative link to the markdown|
| ChildPageBlock | Converts Child Page to markdown. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on the selected output format Command-line argument, we may or many not be exporting the child pages into markdown. In fact, currently, we don't ever export child page objects. In the future, we will have a recursive export format option that will recursively go through a page and export everything.

Copy link
Contributor

Choose a reason for hiding this comment

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

For now, I propose we just ignore child pages. In the future, I think the solution you have outlined is a good one.

@sHermanGriffiths sHermanGriffiths merged commit cae7ace into main Aug 25, 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.

None yet

2 participants