perf: add filter item scroll to the bottom #1988
Conversation
chore: delete useless hook
There was a problem hiding this comment.
Pull Request Overview
This PR adds smooth scroll-to-bottom functionality for filter items and includes a CSS fix for text truncation in hide fields components. The primary purpose is to enhance user experience by automatically scrolling to the bottom when new filter conditions are added.
- Added automatic scroll-to-bottom behavior when adding filter conditions
- Fixed text truncation styling in hide fields component
- Removed unused hook for hidden fields
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/sdk/src/components/filter/BaseFilter.tsx | Added scroll-to-bottom functionality with ref and click handlers |
| packages/sdk/src/components/hide-fields/HideFieldsBase.tsx | Added truncate class to improve text overflow handling |
| apps/nextjs-app/src/features/app/hooks/useHiddenFields.ts | Removed entire unused hook file |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <div | ||
| role="button" | ||
| tabIndex={-1} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter') { | ||
| e.stopPropagation(); | ||
| e.preventDefault(); | ||
| } | ||
| }} | ||
| className={cn('flex justify-start gap-1', footerClassName)} | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| e.preventDefault(); | ||
| setTimeout(() => { | ||
| filterContainerRef?.current?.scrollTo({ | ||
| top: filterContainerRef?.current?.scrollHeight, | ||
| behavior: 'smooth', | ||
| }); | ||
| }, 0); | ||
| }} | ||
| > |
There was a problem hiding this comment.
The div with role='button' should handle all keyboard interactions that a button would. Currently it only handles 'Enter' but should also handle 'Space' key for full accessibility compliance.
Copilot uses AI. Check for mistakes.
| setTimeout(() => { | ||
| filterContainerRef?.current?.scrollTo({ | ||
| top: filterContainerRef?.current?.scrollHeight, | ||
| behavior: 'smooth', | ||
| }); | ||
| }, 0); |
There was a problem hiding this comment.
Using setTimeout with 0ms delay is a code smell that suggests relying on timing for DOM updates. Consider using requestAnimationFrame or ensuring the scroll happens after the DOM update is complete through proper React lifecycle methods.
Copilot uses AI. Check for mistakes.
1 participant
No description provided.