-
Notifications
You must be signed in to change notification settings - Fork 25
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(Compare): Add compare layout #399
base: main
Are you sure you want to change the base?
Conversation
Preview: https://chatbot-pr-chatbot-399.surge.sh A11y report: https://chatbot-pr-chatbot-399-a11y.surge.sh |
01759af
to
f3425a7
Compare
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.
✍️✍️
packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/Chatbot.md
Outdated
Show resolved
Hide resolved
...dule/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx
Outdated
Show resolved
Hide resolved
...dule/patternfly-docs/content/extensions/chatbot/examples/demos/EmbeddedComparisonChatbot.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Erin Donehoo <[email protected]>
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.
mentioned a small nitpick that I last-minute noticed, but nothing that holds this back!
packages/module/patternfly-docs/content/extensions/chatbot/examples/demos/Chatbot.md
Show resolved
Hide resolved
|
||
return ( | ||
<Page masthead={masthead} sidebar={sidebar} isContentFilled> | ||
<div className="pf-chatbot__compare-container"> |
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.
so these classnames are being used in the demo as necessary to create the layout and apply styles. I think they are going to need to be included/called out in the demo docs somehow. Could you maybe add a nolive
code block in the content above the example to outline the structure and classes needed? Something like an outline... like:
<Page or PageSection>
<div className="pf-chatbot__compare-container">
<div className="pf-chatbot__compare-mobile-controls">
...
</div>
</div>
</Page or PageSection>
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.
All set!
45d741e
to
f4eb2ae
Compare
f4eb2ae
to
1eaac2f
Compare
Demo is here for now: https://chatbot-pr-chatbot-399.surge.sh/patternfly-ai/chatbot/overview/demo/comparing-chatbots/
I stuck it under Overview/Demos for now (https://chatbot-pr-chatbot-399.surge.sh/patternfly-ai/chatbot/overview/demo#comparison), but totally happy to move it elsewhere. All I'm shipping as part of the extension for this is a handful of CSS classes.
Will generate screenshots once we decide on docs placement.