-
Notifications
You must be signed in to change notification settings - Fork 1
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
Create CommunityPost component #1011
base: main
Are you sure you want to change the base?
Conversation
🔍 Visual review for your branch is published 🔍Here are the links to: |
I want to spin the design discussion. Why do we force users to click a post to read it? Why can't I see it full? I'm curious, how an average community post in factorial looks like |
lib/experimental/Information/Communities/Post/CommunityPost/index.tsx
Outdated
Show resolved
Hide resolved
createdAt: Date | ||
|
||
title: string | ||
description?: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe content
is a better name. Also if we expect an HTML string there, let's create an alias for this: type HTMLString = string
and use it here for clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that in Figma we have the PostDescription
component I would leave it here as description
, and content
in PostDescription
component not to be redundant there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't implicitly couple the implementation with third party dependency. It should not leave in the factorial one I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is that existing posts and new ones that are created in Factorial use these classes and we should use them to give them some styles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we will rely on the css provided by factorial. it is their responsibility to create the content and manage its style
With the proposed solution we will have to react on any class changes in Factorial content editor, creating a mandatory work to do on our side before the release. We must be agnostic to such issues because it will create time pressure which is bad for any design system.
I propose to keep this styles in Storybook files just to keep things visually clear, but document that the post only provide a shell for the content. The content style itself is the responsibility of the user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we should have control on Factorial One over what classes these posts can contain, we should provide these classes with an editor component as well that is on our side, otherwise we would be missing so much control over how these posts look.
We even have a PostDescription
component in Figma that mandates how mentions should look.
I'd say we should also revisit these styles to guarantee that the colors used are part of the tokens and not some random ones.
That's a very interesting discussion to have, let's talk about it internally with the design team! |
Description
Create new CommunityPost component.
Screenshots
Figma Link
https://www.figma.com/design/pZzg1KTe9lpKTSGPUZa8OJ/Web-Components?node-id=2333-12691&t=9eBZI2wvIjDCe1Tj-4