Skip to content
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: fixed links to specific category in tools section #2190

Closed
wants to merge 12 commits into from
12 changes: 12 additions & 0 deletions components/tools/ToolDashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ export default function ToolDashboard() {
}, 1000);
}, []);

// useEffect to scroll particular div into view
useEffect(() => {
setTimeout(() => {
const path = router?.asPath
const id = path?.slice(path?.indexOf("#") + 1).replace("%20", " ")
const element = document.getElementById(id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using id attribute, can't we use ref here to add the behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can think about this, but the "Jump to Category" dropdown already uses the "id" of each div to scroll to, so wouldn't it be better to use the same strategy for initial scrolling as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually access the DOM elements directly in react. React has it's own virtual DOM feature which enables better performance on the website. That's why I asked to use ref in this case. We used id just to reference the category and link it in dropdown.

if (element) {
element.scrollIntoView({ behavior: 'smooth', block: "nearest", inline: "nearest" })
}
}, 1000)
}, [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}, [])
}, [])

Kindly add a spacing to make it more readable inside codebase (since they are 2 different sections of code).


// useEffect function to enable the close Category dropdown Modal feature when clicked outside of the modal
useEffect(() => {
const checkIfClickOutside = (e) => {
Expand Down
2 changes: 1 addition & 1 deletion components/tools/ToolsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export default function toolsList({ toolsData }) {
<div className="" data-testid="ToolsList-main" >
{Object.keys(toolsData).map((categoryName, index) => {
if(toolsData[categoryName].toolsList.length > 0) return (
<div className='my-8' key={index} id={categoryName}>
<div className='my-8 scroll-m-[6.25rem]' key={index} id={categoryName}>
<Heading typeStyle='heading-md-semibold' className='my-2' >
{categoryName}
</Heading>
Expand Down
39 changes: 13 additions & 26 deletions package-lock.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why there are changes in this file?

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion scripts/build-newsroom-videos.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@ async function buildNewsroomVideos() {
}

buildNewsroomVideos()
module.exports={buildNewsroomVideos}
module.exports={buildNewsroomVideos}