-
Notifications
You must be signed in to change notification settings - Fork 39
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 ActionsColumn to Job Functions table #2101
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: DvoraShechter1 <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2101 +/- ##
==========================================
+ Coverage 39.20% 41.98% +2.78%
==========================================
Files 146 175 +29
Lines 4857 5630 +773
Branches 1164 1409 +245
==========================================
+ Hits 1904 2364 +460
- Misses 2939 3145 +206
- Partials 14 121 +107
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: DvoraShechter1 <[email protected]>
Signed-off-by: DvoraShechter1 <[email protected]>
Signed-off-by: DvoraShechter1 <[email protected]>
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.
Code looks good! Some minor comments:
- mixing links and text in the issue description (first post in the Conversation view) is problematic - in our process the message will be used as the git commit message (which is plain text). A good solution is to list links below the description i.e. see 🌱 Update project to use nodejs 20, npm >=10.5.2 (#2062) #2064 or ✨ Use InfiniteScroller pattern for the Task Drawer #2049
- note that part of your description (
like in the [Stakeholder](https://127.0.0.1/controls/stakeholders) table
) is not accurate - currently (with PR ✨ Update stakeholder groups table to use ActionsColumn #2095 and ✨ Update stakeholders table to use ActionsColumn #1923) we have both stakeholders tables fixed. - you can use
[WIP]
prefix as a way to indicate work-in-progress but GitHub has nice draft PR functionality (check "convert to draft" link below reviewers list)
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!
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
Update the Actions column in the Job Functions table to use PatternFly 5 controls, replacing the PatternFly 4 controls.
Relates to #1318