-
Notifications
You must be signed in to change notification settings - Fork 765
feat(cyclops-ui): Filter Modules by TargetNamespace #666
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
feat(cyclops-ui): Filter Modules by TargetNamespace #666
Conversation
|
@quest-bot loot #640 |
|
hey @petar-cvit @KaradzaJuraj, can you please review this pr in your free time? |
KaradzaJuraj
left a comment
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 @s-vamshi, the filter resets every 10 seconds because we refresh the module list every 10 seconds.
Use the /api/namespaces endpoint to populate the filter data even before you get the modules.
|
|
||
| const { Title } = Typography; | ||
|
|
||
| interface ModuleInterface { |
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'd remove this for now, we'd have to implement it in more places. Let's keep the scope of the PR to just what was defined in the issue.
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.
sure will remove it!
| const Modules = () => { | ||
| const [allData, setAllData] = useState([]); | ||
| const [filteredData, setFilteredData] = useState([]); | ||
| const [allData, setAllData] = useState<ModuleInterface[]>([]); |
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.
Revert to empty array
| const [allData, setAllData] = useState([]); | ||
| const [filteredData, setFilteredData] = useState([]); | ||
| const [allData, setAllData] = useState<ModuleInterface[]>([]); | ||
| const [filteredData, setFilteredData] = useState<ModuleInterface[]>([]); |
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.
Revert to empty array
| return self.indexOf(targetNamespace) === index; | ||
| }); | ||
| setNamespaceFilterData(namespaceData); | ||
| setModuleNamespaceFilter(namespaceData); |
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 cannot use setModuleNamespaceFilter here because it is called every 10 seconds and resets the 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.
true didnt check this my bad, will update
| .get(`/api/modules/list`) | ||
| .then((res) => { | ||
| setAllData(res.data); | ||
| populateNamespaceData(res.data); |
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.
Move the call of this function outside the fetchModules loop.
In this useEffect, before the fetchModules function, create a call towards /api/namespaces. This will return all available namespaces with which you can populate both the namespaceFilterData and the moduleNamespaceFilter
|
hey @KaradzaJuraj thanks for reviewing this, will be pushing the changes soon! |
|
hey @KaradzaJuraj made the changes as mentioned can you please review them in your free time? |
KaradzaJuraj
left a comment
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 @s-vamshi, just a few minor tweaks and it should be good to go!
|
hey @KaradzaJuraj, done with the changes! please review them in your free time!! |
KaradzaJuraj
left a comment
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.
Thanks @s-vamshi, looks great! 🚀
closes #640
📑 Description
✅ Checks
ℹ Additional context
screen-capture.1.webm