-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
fix(UI/advanced-mode): collapse animation more smooth #9803
base: main
Are you sure you want to change the base?
fix(UI/advanced-mode): collapse animation more smooth #9803
Conversation
Author: Julie Wu <[email protected]> Date: Wed Jan 22 19:08:02 2025 +0100
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
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.
PR Summary
This PR enhances the collapse animation smoothness in advanced settings by adjusting styling properties in the AdvancedSettingsWrapper and form components.
- Added
padding-bottom
using theme spacing inpackages/twenty-front/src/modules/settings/components/AdvancedSettingsWrapper.tsx
to prevent content cutoff during animation - Modified
StyledAdvancedSettingsSectionInputWrapper
inpackages/twenty-front/src/modules/settings/data-model/objects/forms/components/SettingsDataModelObjectAboutForm.tsx
withz-index: -1
andposition: relative
for better stacking context - Removed
gap
property fromStyledAdvancedSettingsSectionInputWrapper
to eliminate spacing interference with animations
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
flex: 1; | ||
z-index: -1; | ||
position: relative; |
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.
logic: Setting z-index: -1 could cause issues with interactive elements inside this wrapper becoming unclickable if there are other elements with a higher z-index that overlap
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.
left a comment
@@ -5,6 +5,7 @@ import { useRecoilValue } from 'recoil'; | |||
import { AnimatedExpandableContainer, IconPoint, MAIN_COLORS } from 'twenty-ui'; | |||
|
|||
const StyledAdvancedWrapper = styled.div` | |||
padding-bottom: ${({ theme }) => theme.spacing(4)}; |
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.
@yukirine I think this might cause regressions somewhere else.
Could u verify all the places where this wrapper is being used we might have unwanted padding-bottom.
Also please mention the issue # in description :) thanks |
@ehconitin Hey |
Author: Julie Wu [email protected]
Date: Wed Jan 22 19:08:02 2025 +0100