-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Made bubble height width configurable through props. #945
Conversation
WalkthroughThe changes introduced in this pull request primarily focus on enhancing the customization of the chat bubble component. The Changes
WalkthroughThe changes introduced in this pull request primarily focus on enhancing the customization of the Changes
TipsChat with CodeRabbit Bot (
|
@prateekkalra is attempting to deploy a commit to the Typebot Team on Vercel. A member of the Team first needs to authorize it. |
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.
Review Status
Actionable comments generated: 1
Files selected for processing (2)
- packages/embeds/js/src/features/bubble/components/Bubble.tsx (3 hunks)
- packages/embeds/js/src/features/bubble/types.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/embeds/js/src/features/bubble/types.ts
Additional comments (Suppressed): 2
packages/embeds/js/src/features/bubble/components/Bubble.tsx (2)
20-24: The
BubbleProps
interface has been extended to include optionalheight
andwidth
properties. This allows for greater customization of the chat bubble's dimensions. Ensure that all uses of theBubble
component throughout the codebase are compatible with these new properties.166-167: The
w-full
class has been removed from thebot
part's class list. This could affect the chat bubble's width on small screens. Ensure that this change does not negatively impact the chat bubble's appearance or functionality on small screens.
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.
The difference in images might be due to screen resolution. |
Width is ok in my screenshot, it's the height that is not! Please checkout the main branch to check the default styling 🙏 |
Sure @baptisteArno |
Hi @baptisteArno |
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.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- packages/embeds/js/src/features/bubble/components/Bubble.tsx (3 hunks)
Additional comments (Suppressed): 3
packages/embeds/js/src/features/bubble/components/Bubble.tsx (3)
23-24: The addition of
height
andwidth
properties to theBubbleProps
interface allows for greater customization of the chat bubble's dimensions. This is a good change as it enhances the flexibility of the component's usage. However, it would be beneficial to add a comment explaining what units these properties should be in (e.g., pixels, em, etc.) to avoid any confusion for future developers.150-154: The
height
andwidth
properties are used to set theheight
andwidth
styles of thebot
div. If the properties are not provided, fallback values are used. This is a good approach as it ensures that the chat bubble will have default dimensions if no specific dimensions are provided. However, thewidth
fallback value includes an!important
flag, which is generally discouraged as it can make debugging more difficult by breaking the natural cascading in your stylesheets. Consider removing this flag if it's not necessary.165-166: The
width
property is removed from the class string. This is consistent with the addition of thewidth
property to theBubbleProps
interface and the use of this property to set thewidth
style of thebot
div. However, ensure that this change does not affect the layout of the chat bubble in unexpected ways, especially in different screen sizes and resolutions.
@baptisteArno You were right about the height issue. It was looking same for me, because of the smaller resolution in my laptop. Pushed a fix to align the default height with main branch. Removed max-height, since it's already been taken care by the tailwind class. |
Sorry the issue lacked some context and clear scope. I'll close this as it does not fully satisfy what I was thinking about 🙏 |
/tip 10 |
🎉🎈 @prateekkalra has been awarded $10! 🎈🎊 |
/claim #458
Made bubble height width configurable through props.
Please find default height width screenshot below.
Summary by CodeRabbit