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

feat(comment): basic angular wrapper implementation #2866

Merged
merged 20 commits into from
Oct 11, 2024

Conversation

Supamiu
Copy link
Contributor

@Supamiu Supamiu commented Jun 18, 2024

Description

The comment component is used to display comments or discussions on blocks

Each block is composed of informations about the author (avatar, a TemplateRef and authorName, a PortalContent (string | TemplateRef)) and handles comments that are placed inside of it as child lu-comment components.



@Supamiu Supamiu added 👥 Front Requires Angular skills 🔖✨ Feature New feature (even a very small one) 📐 Angular wrapper Provide an Angular wrapper to a HTML/CSS component labels Jun 18, 2024
@c-3po c-3po bot added the 📖 Documentation changes Requires a Prisme update label Jun 18, 2024
@LuccaIntegration
Copy link

@CCNET-iLucca
Copy link

Tests d'interfaces

@LuccaIntegration
Copy link

@CCNET-iLucca
Copy link

Tests d'interfaces

Supamiu and others added 6 commits August 29, 2024 15:55
…r-inte' into feat/comment-wrapper

# Conflicts:
#	packages/ng/comment/comment-block/comment-block.component.html
#	packages/ng/comment/comment-block/comment-block.component.ts
#	packages/ng/comment/comment/comment.component.html
#	packages/ng/comment/comment/comment.component.ts
#	stories/documentation/texts/comment/angular.stories.ts
@LuccaIntegration
Copy link

@Supamiu Supamiu marked this pull request as ready for review August 30, 2024 08:45
@Supamiu Supamiu requested review from a team as code owners August 30, 2024 08:45
@Supamiu Supamiu added this to the 18.3 milestone Aug 30, 2024
@CCNET-iLucca
Copy link

Tests d'interfaces

@LuccaIntegration
Copy link

@LuccaIntegration
Copy link

@@ -0,0 +1,3 @@
<ol class="commentWrapperChat">
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this shouldn't be considered as a new component as I heard Clément speaking about "Chatbox". I'll investigate.

packages/scss/src/components/comment/component.scss Outdated Show resolved Hide resolved

&.mod-WrapperAvatar ~ &:not(.mod-WrapperAvatar) {
padding-left: var(--components-comment-content-margin);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason behind this deletion?
It seems to break the comment's wrap (with avatar only on first comment) :
image

@jeremie-lucca jeremie-lucca added the 💥 Breaking change Requires actions on products side (even a very small one) label Oct 7, 2024
@LuccaIntegration
Copy link

@CCNET-iLucca
Copy link

Tests d'interfaces

@jeremie-lucca jeremie-lucca added the 🎨 Design sync Discussions with design are required label Oct 7, 2024
@jeremie-lucca
Copy link
Contributor

(Waiting design feedback on components naming before mergin')

@LuccaIntegration
Copy link

@LuccaIntegration
Copy link

jeremie-lucca
jeremie-lucca previously approved these changes Oct 11, 2024
@LuccaIntegration
Copy link

@jeremie-lucca jeremie-lucca merged commit 835878b into rc Oct 11, 2024
3 checks passed
@jeremie-lucca jeremie-lucca deleted the feat/comment-wrapper branch October 11, 2024 08:01
@jeremie-lucca jeremie-lucca mentioned this pull request Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📖 Documentation changes Requires a Prisme update 🔖✨ Feature New feature (even a very small one) 💥 Breaking change Requires actions on products side (even a very small one) 🎨 Design sync Discussions with design are required 👥 Front Requires Angular skills 📐 Angular wrapper Provide an Angular wrapper to a HTML/CSS component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants