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

Markdown block group component#48 #75

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

Conversation

RobH0
Copy link
Contributor

@RobH0 RobH0 commented Mar 11, 2025

  • Created new MarkdownBlockGroup component.
  • Wrote storybook stories showing example renderings of MarkdownBlockGroup components.

Resolves #48

RobH0 added 17 commits August 6, 2024 12:18
@RobH0 RobH0 requested a review from ddfridley March 11, 2025 22:40
Copy link
Contributor

@ddfridley ddfridley left a comment

Choose a reason for hiding this comment

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

Very nice. Unfortunately things have evolved a little since the issue was written.

Please use block.js as the outer wrapper for this component, see how it is done in markdown-block.js

Then, instead of importing MarkdownBlock, please edit markdown-block.js and export MarkdownWithImage and then use that (so that the items in the group aren't wrapped with the Block margins.

Then, the width of this component must fit it's parent div, currently this component renders a fixed width, causing the horizontal scrollbar to appear when the window window is thinner, as below.

image

Let the blocks in the group get skinnier and then wrap into rows. Check out how that works with articles at https://enciv.org/articles and see article-thumbnails-block.jsx for how that did it.

Lastly - it's my bad but lets eliminate the cols prop and just use blocks.length. I will update the issue description.

Thanks for working on this. I'm surprised you turned it around so quickly after the dependency was finished.

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.

MarkdownBlockGroup component
2 participants