-
-
Notifications
You must be signed in to change notification settings - Fork 681
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: navigates user to the relevant category when URL is provided #2786
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.
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. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-2786--asyncapi-website.netlify.app/ |
@@ -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-96 scroll-mb-96' key={index} id={categoryName}> |
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.
Why there is a change in the design ??
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.
When we select the category from the dropdown directly, it takes us to the heading of that category, but it does not happen if we directly enter the URL. Hence this change in the CSS.
const urlPath = router.asPath; | ||
const elementIndex = urlPath.lastIndexOf('#'); | ||
if (elementIndex !== -1) { | ||
const element = urlPath.substring(elementIndex + 1).replace(/%20/g, ' '); |
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.
I don't think we should use hardcode functions.
Is there any other way of doing this??
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.
I didn't get what you mean by hardcoded functions. Could you please elaborate on it?
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.
I mean how we are checking the slug and replacing it
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.
We are looking at the specific component of the URL i.e. the category. Hence for URLs like domain/tools#Frameworks
I try to extract the word Frameworks
using elementIndex
.
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.
try running this url on your local host https://localhost:3000/tools#Documentation%20Generators
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.
asyncapi.mp4
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.
@sambhavgupta0705 is the PR good to be merged?
@helios2003 for thisss issue we are considering #2790 as it solves the issue in a better way |
Description
On entering the URL on the search bar, the user correctly gets navigated to the relevant category.
After the fix
asyncapi-issue.mp4
Related issue(s)
Fixes #2181