-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[v16] Fix Integration Button styles #50283
Conversation
If a user could list integrations, and they could NOT create integrations, AND integrations existed, this would cause the disabled 'enroll new integration' button to be smashed to the left of the header (this is due to how its wrapped in an optional Hover Tooltip). This PR just adds some space-between flex property to the header to ensure the button stays on the right side. It also moves the HoverTooltip position to the bottom because it looks better
@@ -36,6 +36,7 @@ export function IntegrationsAddButton({ | |||
|
|||
return ( | |||
<HoverTooltip | |||
position="bottom" |
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.
No <FeatureHeader justifyContent="space-between">
in v16?
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.
Looking into it now
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, this doesn't need to go to v16. I forgot I fixed it in that backport due to other conflicts. I'll close this. The position prop doesn't exist in v16 (i guess its changes never got backported).
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.
@avatus FWIW, v16 doesn't have position="bottom"
.
<HoverTooltip | |
tipContent={ | |
canCreateIntegrations ? null : ( | |
<MissingPermissionsTooltip | |
requiresAll={false} | |
missingPermissions={missingPermissions} | |
/> | |
) | |
} | |
> | |
<Button |
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.
I'm going to ask around to understand why the recent changes to HoverTooltip
didn't make it back to v16
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Pull request was closed
Backport #50258 to branch/v16