feat: enhance search modal with smooth opening and closing transition #3647
Conversation
auraticabhi
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe changes add smooth transition animations for the Algolia modal. A new state variable ( Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3647 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 732 732
=========================================
Hits 732 732 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/AlgoliaSearch.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 180000ms (2)
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Lighthouse CI
🔇 Additional comments (2)
components/AlgoliaSearch.tsx (2)
36-40: LGTM! Interface change is backward compatible.The updated
onCloseprop type now correctly handles both programmatic and event-based closures.
127-136: LGTM! Well-structured animation implementation.The implementation follows best practices:
- Separate transition durations for backdrop and modal
- Proper event propagation handling
- Smooth scale and opacity transitions
auraticabhi
changed the title
auraticabhi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/AlgoliaSearch.tsx (1)
129-158: LGTM: Clean transition implementation.The modal transitions are well-implemented with:
- Proper backdrop handling
- Event propagation control
- Matching transition durations
Consider extracting the transition durations into constants to ensure consistency between timeouts and CSS:
+const ENTRY_DELAY = 10; +const EXIT_DELAY = 300; +const BACKDROP_DURATION = 200; +const MODAL_DURATION = 300; function AlgoliaModal({ onClose, initialQuery, indexName }: AlgoliaModalProps) { // ... useEffect(() => { - const timer = setTimeout(() => setIsVisible(true), 10); + const timer = setTimeout(() => setIsVisible(true), ENTRY_DELAY); return () => clearTimeout(timer); }, []); const handleClose = () => { setIsVisible(false); - const timer = setTimeout(onClose, 300); + const timer = setTimeout(onClose, EXIT_DELAY); return () => clearTimeout(timer); }; return createPortal( <div - className={`fixed inset-0 z-50 bg-black/50 transition-all duration-200 ${ + className={`fixed inset-0 z-50 bg-black/50 transition-all duration-${BACKDROP_DURATION} ${ isVisible ? 'scale-100 opacity-100' : 'scale-95 opacity-0' }`} onClick={handleClose} > <div - className='rounded-lg bg-transparent p-6 shadow-lg transition-all duration-300' + className={`rounded-lg bg-transparent p-6 shadow-lg transition-all duration-${MODAL_DURATION}`} onClick={(e) => e.stopPropagation()} >
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/AlgoliaSearch.tsx(1 hunks)
🧰 Additional context used
🪛 ESLint
components/AlgoliaSearch.tsx
[error] 118-118: Expected blank line after variable declarations.
(newline-after-var)
[error] 119-119: Expected blank line before this statement.
(padding-line-between-statements)
[error] 119-119: Delete ·
(prettier/prettier)
[error] 121-121: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 121-121: Delete ··
(prettier/prettier)
[error] 124-124: Expected blank line after variable declarations.
(newline-after-var)
[error] 125-125: Expected blank line before this statement.
(padding-line-between-statements)
[error] 125-125: Delete ·
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
components/AlgoliaSearch.tsx
[error] 118-118: Expected blank line after variable declarations. newline-after-var
[error] 119-119: Expected blank line before this statement. padding-line-between-statements
[error] 119-119: Delete · prettier/prettier
[error] 121-121: Trailing spaces not allowed. no-trailing-spaces
[error] 121-121: Delete ·· prettier/prettier
[error] 124-124: Expected blank line after variable declarations. newline-after-var
[error] 125-125: Expected blank line before this statement. padding-line-between-statements
[error] 125-125: Delete · prettier/prettier
🔇 Additional comments (1)
components/AlgoliaSearch.tsx (1)
36-40: LGTM: Props type update.The
onCloseprop type has been correctly updated to handle the backdrop click event.
Fixed formatting issues. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/AlgoliaSearch.tsx (2)
115-121: Extract delay value as a constant.Consider extracting the hardcoded delay value as a named constant at the top of the file for better maintainability.
+const MODAL_ENTRY_DELAY_MS = 10; + function AlgoliaModal({ onClose, initialQuery, indexName }: AlgoliaModalProps) { const router = useRouter(); const [isVisible, setIsVisible] = useState(false); useEffect(() => { - const timer = setTimeout(() => setIsVisible(true), 10); // Small delay for smooth entry + const timer = setTimeout(() => setIsVisible(true), MODAL_ENTRY_DELAY_MS); // Small delay for smooth entry return () => clearTimeout(timer); // Cleanup function }, []);
123-128: Sync animation duration with CSS transition.Extract the hardcoded delay value as a constant and ensure it matches the CSS transition duration (200ms for opacity and 300ms for transform).
+const MODAL_EXIT_DELAY_MS = 300; // Match the longest CSS transition duration + function AlgoliaModal({ onClose, initialQuery, indexName }: AlgoliaModalProps) { // ... const handleClose = () => { setIsVisible(false); - const timer = setTimeout(onClose, 300); // Delay closing to allow animation to finish + const timer = setTimeout(onClose, MODAL_EXIT_DELAY_MS); // Delay closing to allow animation to finish return () => clearTimeout(timer); // Cleanup function };
| <div | ||
| className={`fixed inset-0 z-50 bg-black/50 transition-all duration-200 ${ | ||
| isVisible ? 'scale-100 opacity-100' : 'scale-95 opacity-0' | ||
| }`} | ||
| onClick={handleClose} | ||
| > | ||
| <div | ||
| className='rounded-lg bg-transparent p-6 shadow-lg transition-all duration-300' | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| <DocSearchModal | ||
| initialQuery={initialQuery} | ||
| initialScrollY={window.scrollY} | ||
| searchParameters={{ distinct: 1 }} | ||
| placeholder={indexName === DOCS_INDEX_NAME ? 'Search documentation' : 'Search resources'} | ||
| onClose={handleClose} | ||
| indexName={indexName} | ||
| apiKey={API_KEY} | ||
| appId={APP_ID} | ||
| navigator={{ | ||
| navigate({ itemUrl }) { | ||
| handleClose(); | ||
| router.push(itemUrl); | ||
| } | ||
| }} | ||
| hitComponent={Hit} | ||
| transformItems={transformItems} | ||
| /> | ||
| </div> | ||
| </div>, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance modal accessibility.
The modal implementation should include proper accessibility features:
- Add
role="dialog"andaria-modal="true"to the modal wrapper. - Add
aria-labelto describe the modal's purpose. - Trap focus within the modal when open.
<div
className={`fixed inset-0 z-50 bg-black/50 transition-all duration-200 ${
isVisible ? 'scale-100 opacity-100' : 'scale-95 opacity-0'
}`}
+ role="dialog"
+ aria-modal="true"
+ aria-label="Search documentation"
onClick={handleClose}
>Consider using a focus trap library like focus-trap-react to manage keyboard focus:
import { FocusTrap } from 'focus-trap-react';
// Wrap modal content
<FocusTrap active={isVisible}>
<div className="rounded-lg bg-transparent p-6 shadow-lg transition-all duration-300">
{/* ... */}
</div>
</FocusTrap>
Description
Related issue(s)
Fixes #3646
Summary by CodeRabbit