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

Create CommunityPost component #1011

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

Conversation

sauldom102
Copy link
Contributor

@sauldom102 sauldom102 commented Jan 3, 2025

Description

Create new CommunityPost component.

Screenshots

Screenshot 2025-01-03 at 12 18 12 Screenshot 2025-01-03 at 12 18 29 Screenshot 2025-01-03 at 12 18 40 Screenshot 2025-01-03 at 12 19 29 Screenshot 2025-01-03 at 13 57 36 Screenshot 2025-01-03 at 12 18 59

Figma Link

https://www.figma.com/design/pZzg1KTe9lpKTSGPUZa8OJ/Web-Components?node-id=2333-12691&t=9eBZI2wvIjDCe1Tj-4

Copy link

github-actions bot commented Jan 3, 2025

🔍 Visual review for your branch is published 🔍

Here are the links to:

@sauldom102 sauldom102 marked this pull request as ready for review January 3, 2025 12:58
@sauldom102 sauldom102 requested a review from a team as a code owner January 3, 2025 12:58
@nlopin
Copy link
Contributor

nlopin commented Jan 3, 2025

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

createdAt: Date

title: string
description?: string
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor

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

Copy link
Contributor Author

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.

Screenshot 2025-01-03 at 17 09 24

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.

@sauldom102
Copy link
Contributor Author

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

That's a very interesting discussion to have, let's talk about it internally with the design team!

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