Skip to content

Conversation

@ocornec
Copy link
Contributor

@ocornec ocornec commented May 19, 2025

Closes #

{{short description}}

Changelog

New

  • {{new thing}}

Changed

  • {{change thing}}

Removed

  • {{removed thing}}

Testing / Reviewing

{{ Add descriptions, steps or a checklist for how reviewers can verify this PR works or not }}

@ocornec ocornec requested review from a team as code owners May 19, 2025 14:10
@ocornec ocornec requested a review from a team May 19, 2025 14:10
@ocornec ocornec requested a review from a team as a code owner May 19, 2025 14:10
@ocornec ocornec requested review from devadula-nandan, kennylam, makafsal and tay1orjones and removed request for a team May 19, 2025 14:10
@netlify
Copy link

netlify bot commented May 19, 2025

Deploy Preview for carbon-labs-web-components failed. Why did it fail? →

Name Link
🔨 Latest commit 1353588
🔍 Latest deploy log https://app.netlify.com/projects/carbon-labs-web-components/deploys/682b3bd9f9eb090008400836

@netlify
Copy link

netlify bot commented May 19, 2025

Deploy Preview for carbon-labs-react ready!

Name Link
🔨 Latest commit 1353588
🔍 Latest deploy log https://app.netlify.com/projects/carbon-labs-react/deploys/682b3bd94854d4000840c2b0
😎 Deploy Preview https://deploy-preview-600--carbon-labs-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

# Other packages
/packages/react/src/components/AnimatedHeader/ @carbon-design-system/animated-header-devs
/packages/react/src/components/Processing/ @carbon-design-system/processing-devs
<<<<<<< Updated upstream
Copy link
Member

Choose a reason for hiding this comment

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

probably should clean this up

Copy link
Member

Choose a reason for hiding this comment

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

Let's delete this file here

Copy link
Contributor

@jlongshore jlongshore left a comment

Choose a reason for hiding this comment

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

Just a few small things

(example) =>
html`
<h3>${example.title}</h3>
<br />
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer styles to add spacing rather than break tags

this.scrollDiv = this; /*this.shadowRoot?.querySelector(
'.clabs--chat-messages-container'
);
);*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove dead code

scrollTarget = this._previousScrollHeight;
}
console.log(this.scrollDiv);
this.scrollDiv?.scrollTo({
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming these console log's should be removed as well

: ''}
${dockingEnabled ? clabsPrefix + '--chat-messages-container-docked' : ''}">
: ''} ${dockingEnabled
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little hard to follow with all the nested ternary's here - is there a better place I'm wondering... maybe before the return you could have an array and for each of those evaluated class names, if supported ( streamResponses, dockingEnabled... etc - is truthy ) push the class name... then your class assignment here you could just Array.split(" ").

*/
_buildOptions() {
const whiteTheme = {
/*const whiteTheme = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove dead code

@tay1orjones tay1orjones removed request for a team and tay1orjones June 12, 2025 15:50
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