-
Notifications
You must be signed in to change notification settings - Fork 34
feat(MessageBar): pulled in compass styles #749
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
2e208a6
feat(MessageBar): pulled in compass styles
thatblindgeye bbbce25
Removed console
thatblindgeye 5107464
Added beta flags
thatblindgeye 66dfc98
Updated example verbiage
thatblindgeye 37066d6
Imported css from react-styles
thatblindgeye File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
15 changes: 15 additions & 0 deletions
15
...ernfly-docs/content/extensions/chatbot/examples/UI/ChatbotMessageBarIndicatorThinking.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import { FunctionComponent, useState } from 'react'; | ||
| import { MessageBar } from '@patternfly/chatbot/dist/dynamic/MessageBar'; | ||
|
|
||
| export const ChatbotMessageBarIndicatorThinking: FunctionComponent = () => { | ||
| const [isThinking, setIsThinking] = useState(false); | ||
| const handleSend = (_message: string | number) => { | ||
| setIsThinking(true); | ||
|
|
||
| setTimeout(() => { | ||
| setIsThinking(false); | ||
| }, 10000); | ||
| }; | ||
|
|
||
| return <MessageBar onSendMessage={handleSend} hasAiIndicator isThinking={isThinking} />; | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
@mcoker this is to prevent box-shadows from blocking out the animation. Was also thinking we could apply some static class to indicate "this message bar will think" ("pf-m-think", sorta like how PF has pf-m-progress and pf-m-in-progress?), which we could have some prop for. Would we want to recommend something like that, though?
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... just curious - do you know why that style was created as a box shadow and not a border? I wonder if it's because the border width needs to change dynamically and you don't want it to cause the message bar to shift?
If we want to keep that border from showing up when it has
-m-thinking(or it's wrapped in something with-m-thinking), this looks like a good candidate for a plain variation since we're effectively creating a separate border for it. Would that work?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 were issues that arose when using a border I believe, @rebeccaalpert could explain that better, but yeah the border width changing would at least be one reason.
Do you mean apply a plain variation to the MessageBar wrapper? Then from the consumer end, they'd need to pass aprop to apply that plain styling/class as well as the thinking class?
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.
Oh sorry I had this and the
ai-indicatorclass confused and thought this was a border style. I think your style block is fine. Having a "will think" class could be good, but the only benefit I can think of is that if we end up needing a.pf-m-thinkdown the road and users will need to add it to get the correct thinking classes, that's already a requirement now (not breaking) instead of a new requirement later on (probably breaking?).Another thing it could do is create a single
.pf-m-thinkclass where you can bundle all of the styles that need to change for.pf-v6-m-thinkinginstead of sprinkling:not(.pf-v6-m-thinking)throughout existing styles, but you could do that currently by just having a block that uses.pf-v6-m-thinkinglike thisCan you think of any other benefits of
.pf-m-thinkthat I'm not considering?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.
Not off the top of my head no, was just curious about this specific scenario really.
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.
Cool, I would just stick with what you have then. FWIW on the border/box-shadow question, @srambach left a comment about that here - https://github.com/patternfly/chatbot/pull/734/files#r2482294423. So we'll probably want to make this a border instead of a box shadow at some point. And if the problem was with the border-width dynamically changing and impacting the message bar layout, you could use a pseudo element to create the border and absolutely position it relative to the message bar.
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.
Cool 🫘 , sounds good!