-
Notifications
You must be signed in to change notification settings - Fork 43
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
✨ Add Dependencies page & drawer #1077
Conversation
ibolton336
commented
Jun 29, 2023
•
edited
Loading
edited
- Adds the dependencies page & dependencies drawer from these mockups
- Uses the existing server side filter/sort method for both the dependencies and dependencies applications components
b079fed
to
29a1692
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1077 +/- ##
==========================================
+ Coverage 44.04% 44.08% +0.03%
==========================================
Files 177 177
Lines 4493 4498 +5
Branches 1007 1007
==========================================
+ Hits 1979 1983 +4
- Misses 2503 2504 +1
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
a605691
to
3a57ba8
Compare
|
||
export const Dependencies: React.FC = () => { | ||
const { t } = useTranslation(); | ||
|
||
const allAffectedApplicationsFilterCategories = | ||
useSharedAffectedApplicationFilterCategories(); |
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.
Changed this shared hook name since we are using it in dependencies. Open to name suggestions if this one is confusing.
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.
This name works for me, I can't really think of anything better than "affected application filter categories" for "filters on application properties that aren't for the app inventory page" 🙃
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.
LGTM once we address the CI issue! Nice.
@ibolton336 the CI error here looks to be a conflict with the PF upgrade:
The old TableComposable is now just Table. |
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.
Spoke too soon, a couple tiny things:
Signed-off-by: ibolton336 <ibolton@redhat.com>
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.
LGTM
Fixes a bug @ibolton336 found where filters weren't working on the Single Application tab of the Issues page. Looks like this param was removed in #1077 Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>