Skip to content
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(Settings): Add settings layout #360

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

rebeccaalpert
Copy link
Member

@rebeccaalpert rebeccaalpert commented Dec 6, 2024

Stuck this as a separate component within the chatbot: https://chatbot-pr-chatbot-360.surge.sh/patternfly-ai/chatbot/ui/react/settings/

Docs: https://chatbot-pr-chatbot-360.surge.sh/patternfly-ai/chatbot/ui/#settings

I added a dropdown that can also toggle it, but unsure if we want other stuff in this demo. I know Nicole generally wants them simple, but we don't have any guidelines for this yet.

@patternfly-build
Copy link

patternfly-build commented Dec 6, 2024

@rebeccaalpert rebeccaalpert linked an issue Dec 6, 2024 that may be closed by this pull request
@rebeccaalpert rebeccaalpert force-pushed the settings branch 4 times, most recently from 0466ab9 to 7907f99 Compare December 17, 2024 17:30
@rebeccaalpert rebeccaalpert changed the title Draft: feat(Settings): Add settings layout feat(Settings): Add settings layout Dec 17, 2024
Copy link

@kaylachumley kaylachumley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks pretty good! a few weeks back I know we discussed having the LM toggle be in the settings page instead of the header as an option for some products. Do we want to add that as a prop or something to optionally turn on and off?
Screenshot 2024-12-17 at 2 01 36 PM

@rebeccaalpert
Copy link
Member Author

They should be able to fill in whatever they would like! I can add this to the demo if you'd like it in there though.

Copy link
Contributor

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A side thought that I had (which isn't a blocker) is if we should add one of the "back to bottom" buttons to the default overlay display?

image

Just because I couldn't initially tell that there were more items to scroll to, due to the scroll bar only appearing once you start scrolling:

image

Kind of a small thing that may not actually be a confusion for any real users, but thought I'd mention it to see thoughts/in case it's not a difficult add!

Copy link
Contributor

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

content is good 👍

@kaylachumley
Copy link

A side thought that I had (which isn't a blocker) is if we should add one of the "back to bottom" buttons to the default overlay display?
image

Just because I couldn't initially tell that there were more items to scroll to, due to the scroll bar only appearing once you start scrolling:
image

Kind of a small thing that may not actually be a confusion for any real users, but thought I'd mention it to see thoughts/in case it's not a difficult add!

Let's add a scroll bar that shows on default if there is extra items

@rebeccaalpert rebeccaalpert force-pushed the settings branch 2 times, most recently from b18c96b to 0bd86b1 Compare December 17, 2024 21:10
@rebeccaalpert
Copy link
Member Author

Scroll bar added @kaylachumley @edonehoo!

tooltipContent = 'Close'
}: ChatbotHeaderCloseButtonProps) => (
<div className={`pf-chatbot__menu ${className}`}>
<Tooltip content={tooltipContent} position="bottom" {...tooltipProps}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to deliver a whole new close button component? what about this one is different than other buttons?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Kayla and the Lightspeeds might want a close button for the general chatbot if they're going to be toggling it from other locations. This way it can be used for Settings or in the main chatbot itself. We don't have a universal chatbot button yet that incorporates all the header button styles - I'd want to do that maybe as we have time.

@nicolethoen nicolethoen merged commit 55452b5 into patternfly:main Dec 17, 2024
5 checks passed
Copy link

🎉 This PR is included in version 2.2.0-prerelease.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v6 chatbot - Add settings bar
5 participants