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

[F] Add markdown blocks to RG homepage #3807

Merged
merged 6 commits into from
Feb 7, 2025
Merged

[F] Add markdown blocks to RG homepage #3807

merged 6 commits into from
Feb 7, 2025

Conversation

1aurend
Copy link
Contributor

@1aurend 1aurend commented Jan 29, 2025

Also, fixes..

  1. Missing focus styles for action buttons in dnd list
  2. Keyboard move controls for collectables

@1aurend 1aurend requested a review from dananjohnson January 30, 2025 00:41
Copy link
Member

@dananjohnson dananjohnson left a comment

Choose a reason for hiding this comment

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

This is great, Lauren! Thanks for fixing those bugs too. I have two small suggested changes:

  • Can the title be optional on the markdown block? Or include a flag to hide it? It seems like it shouldn't be required.
  • The markdown block should be visually distinct from the categories. How about just making the background transparent and removing the block padding?
Screenshot 2025-01-30 at 3 04 26 PM

@1aurend
Copy link
Contributor Author

1aurend commented Jan 30, 2025

Oh yeah, that looks good.

Re: optional title— it's currently a required property in the api. I can take a look at what side effects there might be of making it optional, and whether a flag would be easier. Perhaps another option could be to assign a title to the markdown blocks on creation, something like markdown_{uid}, always hide the title on markdown blocks, and then update our rte styles, so those block title styles could be replicated by putting an h2 into the markdown body?

Current styles:
Screenshot 2025-01-30 at 3 44 09 PM

@dananjohnson
Copy link
Member

Yeah I like the generated title idea! Since it's a markdown block, they can just add headings if they need them. (And yes to add the RTE styles 👍🏻 )

@1aurend
Copy link
Contributor Author

1aurend commented Feb 7, 2025

@dananjohnson this should be ready to go. Do you want to give it another quick look and unblock merging (or just merge)?

One thing for later— h1 is allowed in markdown in Manifold. Those should probably be converted to h2 by the system. If we wanted to do that, it would either be an api change in the html rendering, or we could add a React markdown renderer and stop using dangerouslySetInnerHTML here.

@dananjohnson
Copy link
Member

Go ahead and merge it. Let's set the h1 question aside for now. I'm not sure it's worth implementing the constraints on our end since it's largely up to content editors to use proper heading structure.

@1aurend 1aurend merged commit 6fd344b into master Feb 7, 2025
3 checks passed
@1aurend 1aurend deleted the mnfld-953 branch February 7, 2025 19:22
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