-
Notifications
You must be signed in to change notification settings - Fork 234
chore(compass-assistant): bump lg-chat packages and apply patches #7246
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the lg-chat packages to newer versions and adds style patches to fix rendering issues in the Assistant chat component. The changes primarily address list styling and heading hierarchy issues while also enabling a previously skipped test.
- Updates 6 lg-chat package versions to incorporate bug fixes and improvements
- Adds comprehensive CSS overrides for proper list styling and heading hierarchy in the Assistant chat
- Enables a previously disabled test for whitespace-only input validation
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
packages/compass-components/package.json | Updates lg-chat package versions to latest patches and minor releases |
packages/compass-assistant/src/assistant-chat.tsx | Adds extensive CSS styling fixes for lists and headings, imports cx utility |
packages/compass-assistant/src/assistant-chat.spec.tsx | Re-enables previously skipped test for whitespace-only input validation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
/** TODO(COMPASS-9751): We're adjusting styling of all the headers to a lower level than the default for chat, this should be updated in Leafygreen as well and removed from our end. */ | ||
'h1, h2, h3, h4, h5, h6': { | ||
margin: 'unset', | ||
color: '#001E2B', |
Copilot
AI
Aug 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded color values should be replaced with design system tokens or CSS variables for better maintainability and consistency.
Copilot uses AI. Check for mistakes.
margin: 'unset', | ||
color: '#001E2B', | ||
fontFamily: | ||
"'Euclid Circular A','Helvetica Neue',Helvetica,Arial, sans-serif", |
Copilot
AI
Aug 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded font family should be replaced with design system tokens for consistency with the overall application styling.
Copilot uses AI. Check for mistakes.
89a2efe
to
dd23582
Compare
/** TODO(COMPASS-9751): We're adjusting styling of all the headers to a lower level than the default for chat, this should be updated in Leafygreen as well and removed from our end. */ | ||
'h1, h2, h3, h4, h5, h6': { | ||
margin: 'unset', | ||
color: '#001E2B', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably have a dark mode variant
'h1, h2, h3, h4, h5, h6': { | ||
margin: 'unset', | ||
color: '#001E2B', | ||
fontFamily: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are font families you can import from leafygreen through compass-component package so that you don't need to write them yourself, but also I guess it doesn't matter that much if that's a temporary hack, mostly just FYI
"version": "2.7.1", | ||
"resolved": "https://registry.npmjs.org/prettier/-/prettier-2.7.1.tgz", | ||
"integrity": "sha512-ujppO+MkdPqoVINuDFDRLClm7D78qbDt0/NR+wp5FqEZOoTNAjPHWj17QRhu7geIHJfcNhRk1XVQmF8Bp3ye+g==", | ||
"version": "2.8.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gribnoysup I might have messed something up but seems like this bumped prettier too, just making sure that's fine. I checked out the package.json and package-lock.json from origin/main and did:
npx compass-scripts update-dependencies "@lg-chat/*" --unsafe-force-install
It does mean I need to run format again on i.e. hadron-document
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah, weird, I don't really see it being a dependency on something in the chain here, but it does indeed gets bumped. I think it's fine to keep it if it's not causing you too much issues
Updates the current lg-chat packages and adds some new style patches that we are going to upstream to Leafygreen to fix both large heading and list issues.