-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fixed record picker loading flickering #12736
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
Conversation
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
Extensive refactoring of record picker components to eliminate loading state flickering through sophisticated state management.
- Introduced new debounced loading states with
MultipleRecordPickerLoadingEffect
andSingleRecordPickerLoadingEffect
using 100ms for initial and 350ms for subsequent loads - Added
CSSWidth
type andRecordPickerLoadingSkeletonList
with varying widths for more natural loading animations - Implemented granular loading states (initial, skeleton, fetch-more) using new component states for better visual transitions
- Removed direct Recoil state management from display components, delegating to dedicated loading effect components
- Made intersection observer height 0px in
MultipleRecordPickerFetchMoreLoader
to prevent layout shifts
17 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
...ages/twenty-front/src/modules/ui/layout/dropdown/components/__stories__/Dropdown.stories.tsx
Show resolved
Hide resolved
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:51119 This environment will automatically shut down when the PR is closed or after 5 hours. |
onEnter={() => { | ||
setSelectedRecordId(undefined); | ||
onRecordSelected(); | ||
}} |
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.
Where possible it's best to create named functions to avoid un-necessary creation of anonymous functions on re-renders.
function onEnter(){
setSelectedRecordId(undefined);
onRecordSelected();
}
...
onEnter={onEnter}
import { SingleRecordPickerComponentInstanceContext } from '@/object-record/record-picker/single-record-picker/states/contexts/SingleRecordPickerComponentInstanceContext'; | ||
import { createComponentStateV2 } from '@/ui/utilities/state/component-state/utils/createComponentStateV2'; | ||
|
||
export const singleRecordPickerShowSkeletonComponentState = |
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.
singleRecordPickerShouldShowSkeletonComponentState
This PR solves a flickering effect on record pickers on the different loading state they can be in. It was designed with @Bonapara to settle on a nice UX feeling. ## Before With fast network (local) : https://github.com/user-attachments/assets/58899934-c705-4b44-b7f6-289045032c11 With slow network : https://github.com/user-attachments/assets/9fb18d86-9da6-4e5d-a83f-00c810fab2dc ## After https://github.com/user-attachments/assets/f4abb40f-5d42-4c46-88ab-aaef4f883f7f Fixes twentyhq#12680 --------- Co-authored-by: Charles Bochet <[email protected]>
This PR solves a flickering effect on record pickers on the different loading state they can be in.
It was designed with @Bonapara to settle on a nice UX feeling.
Before
With fast network (local) :
Before.fast.mov
With slow network :
Before.slow.mov
After
After.mov
Fixes #12680