Skip to content

chore(chat): Ai components doc updates #600

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

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

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 review from kennylam, tay1orjones, makafsal and devadula-nandan and removed request for a team May 19, 2025 14:10
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

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.

@@ -14,7 +14,10 @@
# 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

@@ -211,6 +211,8 @@ export const Showcase = {
html` ${examples.map(
(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

'.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

if (!this.scrollTimeout) {
this.scrollTimeout = setTimeout(() => {
if (this._autoScroll || this.forceScrollDown) {
let scrollTarget = this.scrollDiv?.scrollHeight;
if (this._limitScroll) {
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(" ").

@@ -123,7 +123,7 @@ export default class diagramElement extends LitElement {
* @param {String} mode - fullscreen, test or default
*/
_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

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