Conversation
fix(PM-1448): implemented client side infinite scroll
fix(PM-1468): make notes mandatory
fix(PM-1322): Redirect to requests page once copilot request is created
fix(PM-1448, PM-1322, PM-1468): QA feedbacks
fix(PM-1470): show applications count in tab
fix(PM-1469): added hover actions for application icons
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| !success ? ( | ||
| <> | ||
| <Button primary onClick={onApply} label='Apply' /> | ||
| <Button disabled={!notes?.trim()} primary onClick={onApply} label='Apply' /> |
There was a problem hiding this comment.
The disabled attribute is conditionally set based on notes?.trim(). Ensure that notes is properly initialized and defined in the component to avoid potential runtime errors if notes is undefined or null.
| defaultActive={activeTab} | ||
| onChange={handleTabChange} | ||
| tabs={getCopilotDetailsTabsConfig(isAdminOrPM)} | ||
| tabs={getCopilotDetailsTabsConfig(isAdminOrPM, copilotApplications?.length || 0)} |
There was a problem hiding this comment.
The function getCopilotDetailsTabsConfig now takes an additional parameter copilotApplications?.length || 0. Ensure that this function is updated to handle the new parameter correctly and that it does not break existing functionality. Verify that the logic inside getCopilotDetailsTabsConfig properly utilizes this parameter.
| } | ||
|
|
||
| export const getCopilotDetailsTabsConfig = (isAdminOrPM: boolean): TabsNavItem[] => (isAdminOrPM ? [ | ||
| export const getCopilotDetailsTabsConfig = (isAdminOrPM: boolean, count: number): TabsNavItem[] => (isAdminOrPM ? [ |
There was a problem hiding this comment.
The function getCopilotDetailsTabsConfig now takes an additional parameter count, but there is no validation or default value handling for this parameter. Consider adding validation to ensure count is a non-negative integer.
| import { FC, useContext, useMemo, useState } from 'react' | ||
| import { bind, debounce, isEmpty } from 'lodash' | ||
| import { toast } from 'react-toastify' | ||
| import { useNavigate } from 'react-router-dom' |
There was a problem hiding this comment.
Consider adding useNavigate to the list of hooks used in this component, if not already done, to ensure consistency and clarity in the usage of hooks.
| const { profile }: ProfileContextData = useContext(profileContext) | ||
| const navigate = useNavigate() | ||
|
|
||
| const [formValues, setFormValues] = useState<any>({}) |
There was a problem hiding this comment.
Consider specifying a more precise type for formValues instead of using any to improve type safety and maintainability.
| setFormErrors({}) | ||
| setPaymentType('') | ||
| // Added a small timeout for the toast to be visible properly to the users | ||
| setTimeout(() => { |
There was a problem hiding this comment.
Consider extracting the timeout duration (1000 ms) into a constant with a descriptive name to improve code readability and maintainability.
| const visibleHeight = window.innerHeight | ||
| const fullHeight = document.body.scrollHeight | ||
| const footerElem = document.getElementById('footer-nav-el') | ||
| const footerHeight = (footerElem && footerElem.offsetHeight) || 650 |
There was a problem hiding this comment.
Consider using a more reliable method to get the footer height, as relying on a default value of 650 may not be accurate across different screen sizes or if the footer content changes.
| const fullHeight = document.body.scrollHeight | ||
| const footerElem = document.getElementById('footer-nav-el') | ||
| const footerHeight = (footerElem && footerElem.offsetHeight) || 650 | ||
| if (scrollY + visibleHeight >= fullHeight - (footerHeight + 100)) { |
There was a problem hiding this comment.
The condition scrollY + visibleHeight >= fullHeight - (footerHeight + 100) could be extracted into a well-named variable for better readability and maintainability.
| if (scrollY + visibleHeight >= fullHeight - (footerHeight + 100)) { | ||
| // Scroll near bottom | ||
| setCurrentPage(prev => { | ||
| const maxPages = Math.ceil(matches.length / itemsPerPage) |
There was a problem hiding this comment.
Consider adding a check to ensure itemsPerPage is not zero to prevent potential division by zero errors when calculating maxPages.
2 participants
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1466
https://topcoder.atlassian.net/browse/PM-1448
What's in this PR?
Feedback updates and fixes for the Copilot Portal.