-
-
Notifications
You must be signed in to change notification settings - Fork 750
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: redesigned the dropdown #2645
Conversation
✅ 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-2645--asyncapi-website.netlify.app/ |
Hey @akshatnema @sambhavgupta0705 Please have a look and give your reviews. I'll bring changes further accordingly. |
Friendly reminder. |
hey @Mayaleeeee This PR is related to design. Please have a look and give suggestions. |
@Mayaleeeee PTAL |
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.
Hey @sagarkori143 ,
Selected items from the dropdowns are not properly aligned. Kindly correct them.
Preview:
Hey @akshatnema . I have pushed the requested changes. PTAL. |
config/adopters.json
Outdated
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 are changes in this file?
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.
It got built in my PRs also but I ignored them and didn't add them in commit
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 got these changes from pulling master branch. Should I discard them?
components/tools/FiltersDropdown.js
Outdated
@@ -12,18 +12,18 @@ export default function FiltersDropdown({ dataList = [], checkedOptions = [], se | |||
}; | |||
|
|||
return ( | |||
<div className={twMerge(`max-w-lg flex gap-2 flex-wrap p-2 duration-200 delay-150 ${className}`)} data-testid="FiltersDropdown-div"> | |||
<div className={twMerge(`max-w-lg max-h-[25rem] overflow-y-auto border-4 flex gap-2 flex-col p-2 duration-200 delay-150 ${className}`)} data-testid="FiltersDropdown-div"> |
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 still see that changes I requested before are reverted in the latest change. Kindly update the PR.
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.
@sagarkori143 please remove changes of adopters.json
Okay @sambhavgupta0705 Discarding them right now!! |
Done @sambhavgupta0705 👍 |
screen-20240305-100823.mp4@sagarkori143 while selecting the drop-down options on mobile it is giving a bad experience to users |
@sagarkori143 any update |
Hey @sambhavgupta0705 . Yes I'm trying to find out the reason for that unwanted user experience. |
Great job, @sagarkori143! Can you share the link for the dropdown menu? |
Hey @Mayaleeeee I didn't understand which link you are referring to. Please clarify. |
@Mayaleeeee here is the url https://www.asyncapi.com/tools |
I have gone through all the components of the filters section and checked them for onclick event listeners. Everything seems okay to me and I am not able to find the reason for that unwanted onclick behavior. I need some assistance in this. @sambhavgupta0705 |
@akshatnema can you please look after this one as you have a better knowledge of tools section |
closing this as this PR has conflcts with website migration and we need to think of a good fix for this one |
resolves: #1318