-
Notifications
You must be signed in to change notification settings - Fork 52
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 chosen tags #258
base: main
Are you sure you want to change the base?
Fix chosen tags #258
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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, left you some comments.
for (const exisitingTag of existingTags) { | ||
// we set tags chosen to active and all the others to not | ||
if (existingTags.every(value => value.isActive)) { | ||
existingTags.forEach(existingTag => { |
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.
Instead of putting comment explaining your code you could extract this part into a function and call it
}); | ||
|
||
if (existingTags.every(value => !value.isActive)) { | ||
existingTags.forEach(existingTag => { |
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.
You can save the existingTags.every(value => !value.isActive
from before into a boolean and check it in here (you would probably need to check for !yourBooleanValue)
And you will save one array function
}; | ||
// const handleBtnFilterClick = (filter: ProjectFilter) => { | ||
// handleFilterOptionChange(filter); | ||
// }; | ||
|
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.
Remove the comments please
filters: ProjectFilter[]; | ||
handleFilterOptionChange: any; | ||
} | ||
|
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.
Please change the name to FiltersWindowProps
like we do in the project for components props.
Also don't use any
, change your interface to have defined values.
{t('projectsFilters')}{' '} | ||
</h3> | ||
{/* sort by ProjectPaginationFilter (sortoptions,sortoptionsmapper) */} | ||
<div className="flex gap-4 md:gap-[26px] justify-center md:justify-normal md:items-center"> |
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.
Remove this comment
<div | ||
ref={filterRef} | ||
className="z-[101] absolute -bottom-[23.5rem] md:-bottom-[16.75rem] right-2 px-[34px] py-[27px] rounded-md border border-blue-600 min-w-[300px] md:min-w-0 md:w-[863px] min-h-[260px] md:h-[209px] p-5 bg-gray-50 dark:bg-gray-600" | ||
> |
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.
Do you need the z-[101]
? will it work with smaller z index ?
onChange={handleCategoryOptionSelection} | ||
/> | ||
))} | ||
</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.
Optional - This could be extracted into a separate component, let's say RadioboxList
@DeanYona Can you address the changes anytime soon? |
@Tamir198 can you take over and apply the changes you recommended? |
@Darkmift Maybe later on this week |
I was not avle not find time for this sorry |
i solved it and now the programming languages you pressed are the ones active unless none chosen and then all are active.
Fixes #257