-
Notifications
You must be signed in to change notification settings - Fork 117
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(side-panel): add SidePanelFooter #4002
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: a2146bb The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit a2146bb. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit a2146bb:
|
Size Change: +179 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
Passing run #8325 ↗︎Details:
Review all test suite changes for PR #4002 ↗︎ |
<SidePanelFooter> | ||
<ChatComposerContainer> | ||
<ChatComposer | ||
maxHeight="size10" | ||
config={{ | ||
namespace: "customer-chat", | ||
onError: (e) => { | ||
throw e; | ||
}, | ||
}} | ||
initialValue="Are switch labels required? What about " | ||
placeholder="Chat text" | ||
ariaLabel="A basic chat composer" | ||
/> | ||
<ChatComposerActionGroup> | ||
<Button variant="secondary_icon" size="reset"> | ||
<AttachIcon decorative={false} title="attach files to the message" /> | ||
</Button> | ||
<Button variant="primary_icon" size="reset"> | ||
<SendIcon decorative={false} title="Send" /> | ||
</Button> | ||
</ChatComposerActionGroup> | ||
</ChatComposerContainer> | ||
</SidePanelFooter> |
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.
praise: this example is sooooo good!
<SidePanelHeader> | ||
<Heading as="h3" variant="heading30" marginBottom="space0"> | ||
More filters | ||
</Heading> | ||
</SidePanelHeader> |
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 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.
yeah that's the design, I kind of agree with you though. cc @roxanagomez
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.
+1 this composition could use a separator like the AI composition
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.
@nkrantz yess I agree, let's keep it consistent. I'll update the design.
113fec3
to
215c0bc
Compare
variant?: "default" | "chat"; | ||
/** | ||
* Sets the `justify-content` CSS property. | ||
* @default "flex-start" | ||
* @type {"flex-start" | "flex-end" | "space-between"} | ||
* @memberof SidePanelFooterProps | ||
*/ |
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 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 great!
@@ -64,6 +65,9 @@ export const SidePanelExample = (): React.ReactNode => { | |||
<Separator orientation="horizontal" verticalSpacing="space0" /> | |||
Side Panel content goes here. | |||
</SidePanelBody> | |||
<SidePanelFooter> | |||
Chat composer goes here. |
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.
question: Are these examples meant to say this? Or do they need to be replaced with the actual component?
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 did that intentionally so it didn't get too overwhelming on the docs page but if someone goes to the story file the code is all there
I can add all the code in if you think that'd be better
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'd say you could just make the text generic like "Footer content goes here" since the Body content is generic now.
(I didn't realize initially that the body content wasn't the AI Chat Log anymore)
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.
👍 yeah it was all just taking up too much space
Passing run #8326 ↗︎Details:
Review all test suite changes for PR #4002 ↗︎ |
Changes to AI Chat Log:
Changes to Side Modal:
Changes to Side Panel:
SidePanelFooter
component exported from the Side Panel package for actions or chat composersSidePanelFooter
component in the Side Panel.Changes to Chat Composer:
width=100%"
to ChatComposerContainer to prevent shrinking when text value is removed