-
Notifications
You must be signed in to change notification settings - Fork 53
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
chore(TDOPS-5724): remove react bootstrap #5045
Conversation
Size Change: -684 B (0%) Total Size: 16.6 MB
ℹ️ View Unchanged
|
Co-authored-by: Mathieu Huchet <[email protected]>
@@ -31,7 +33,7 @@ export function decorateWithOverlay( | |||
trigger="click" | |||
rootClose | |||
placement={overlayPlacement} | |||
overlay={<Popover id={overlayId}>{overlayComponent}</Popover>} | |||
overlay={<div id={overlayId}>{overlayComponent}</div>} |
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 is a Popover component in Coral if we want to use it 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.
Yes that's true , but i think we don't really need a popover here as overlaytrigger is already some sort of popover so it's like putting popover into your popover 😂
{...rest} | ||
hideLabel |
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.
We never want to display a label 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.
Good question
From what i've tested it's always integrated as a quick search input without label so if there are some case we need one then we can put a condition here
What is the problem this PR is trying to solve?
remove react bootstrap from
What is the chosen solution to this problem?
Please check if the PR fulfills these requirements
yarn changeset
to a request a release from the CI if wanted.[ ] This PR introduces a breaking change