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(feedback-container): adding feedback-container component #34

Closed
wants to merge 11 commits into from
Closed

feat(feedback-container): adding feedback-container component #34

wants to merge 11 commits into from

Conversation

priyanshu891
Copy link
Contributor

Closes #

Initial Setup of Feedback Container component

Changelog

New

  • Feedback Container component

Changed

  • N/A

Removed

  • N/A

Testing / Reviewing

N/A

Copy link

netlify bot commented Nov 20, 2023

Deploy Preview for carbon-for-ai failed.

Name Link
🔨 Latest commit 958cf1e
🔍 Latest deploy log https://app.netlify.com/sites/carbon-for-ai/deploys/6571dfcd127cac0008dcb265

Copy link
Contributor

github-actions bot commented Nov 20, 2023

DCO Assistant Lite bot: Thanks for your submission! We ask that you all sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text:


I have read the DCO document and I hereby sign the DCO.


0 out of 2 committers have signed the DCO.
@priyanshu Rai
@priyanshu891
Priyanshu Rai seems not to be a GitHub user. You need a GitHub account to be able to sign the DCO. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@ariellalgilmore
Copy link
Member

@priyanshu891 looks like you need to run pnpm install at the root of the project to update the pnpm-lock.yaml

Copy link
Member

@annawen1 annawen1 left a comment

Choose a reason for hiding this comment

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

Have some questions on if these components would live in the chat package instead and the placement of the styles

@@ -0,0 +1,67 @@
/**
* @license
Copy link
Member

Choose a reason for hiding this comment

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

Can these components all be put in the Chat package instead?

* More on how to set up stories at: https://storybook.js.org/docs/web-components/writing-stories/introduction
*/
export default {
title: 'Components/Feedback container/Feedback container',
Copy link
Member

Choose a reason for hiding this comment

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

If we put these components in the chat package, we probably want to change the storybook title here to
Components/Chat/Feedback container so that they are all listed under the Chat component in storybook

packages/feedback/__stories__/input-container.stories.js Outdated Show resolved Hide resolved
@@ -0,0 +1,37 @@
{
"name": "@carbon-labs/feedback",
Copy link
Member

Choose a reason for hiding this comment

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

This may not be needed if we're placing the components within the chat package

import '@carbon/web-components/es/components/button/index.js';

export class FeedbackContainer extends LitElement {
static styles = css`
Copy link
Member

Choose a reason for hiding this comment

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

can these styles be separated out into its own file? (see extended-button component for example)

@priyanshu891
Copy link
Contributor Author

opened new PR

@priyanshu891 priyanshu891 deleted the pr/feedback-container branch February 27, 2024 08:06
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.

3 participants