-
Notifications
You must be signed in to change notification settings - Fork 9
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
host: Add realm icon in attached cards #1086
host: Add realm icon in attached cards #1086
Conversation
This will probably break things visually.
We want the extra padding for the delete icon.
}; | ||
} | ||
|
||
export class CardPill extends Component<CardPillSignature> { |
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's actually another CardPill
component in components/ai-assistant/card-picker/index
. That is the one we're using when cards are attached in the chat input area of the ai-assistant panel. It would be good if we could consolidate them. The difference would be to not to display the remove button. The styles should be the same otherwise.
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.
I couldn’t find anything referencing this in tests… it could be attached with splattributes anyway?
I’m having weird type failures locally…!
@cardstack/boxel-developers Luke suggested that more people check this out as the |
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.
Looks good, just minor note about unnecessary {{let}} nesting
This necessitated adding the ability to not render card chrome so I could insert the icon within the border. A box component now accepts
@displayContainer
, it defaults totrue
but we usefalse
on the attached cards in atom format to include the icon.