-
Notifications
You must be signed in to change notification settings - Fork 0
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
HPC - 8242 Stable version of FTSAdminRewrite without 'Add flow' functionality #420
Conversation
apps/hpc-ftsadmin/src/app/components/filters/filter-pending-flows-table.tsx
Outdated
Show resolved
Hide resolved
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.
Checks have passed and this pull request is ready for manual review
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.
Here are some of the initial comments I have. I just reached 95332f8 and reviewed until file async-autocomplete-field.tsx
. There was a problem when I ran npm install
, so I found what the issue was and resolved it in a separate commit that I pushed.
libs/hpc-ui/src/lib/components/form-fields/async-autocomplete-field.tsx
Outdated
Show resolved
Hide resolved
libs/hpc-ui/src/lib/components/form-fields/async-autocomplete-field.tsx
Outdated
Show resolved
Hide resolved
libs/hpc-ui/src/lib/components/form-fields/async-autocomplete-field.tsx
Outdated
Show resolved
Hide resolved
libs/hpc-ui/src/lib/components/form-fields/async-autocomplete-field.tsx
Outdated
Show resolved
Hide resolved
libs/hpc-ui/src/lib/components/form-fields/async-autocomplete-field.tsx
Outdated
Show resolved
Hide resolved
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
- When switching from
yup
toio-ts
, you have left out some properties ofyup
codecs and removed them fromio-ts
. I've commented about this in few places, but not all. [X] (Commented on one of the comments the reason) - When removing
dependencies
in 5d6a12a, you haven't updated lockfile. [X] - Please update initial comment of this PR and write a proper PR message
- The number of warnings about missing dependencies in
useEffect
hooks has increased - Do we need to keep both GraphQL and REST versions? [X] Removed
- Remove usages of type casting (
as
keyword). I did write about this in few places inline, but not everywhere [X] (I let in some places because of necessity)
apps/hpc-ftsadmin/src/app/components/filters/filter-flows-REST-table.tsx
Outdated
Show resolved
Hide resolved
apps/hpc-ftsadmin/src/app/components/filters/filter-flows-REST-table.tsx
Outdated
Show resolved
Hide resolved
apps/hpc-ftsadmin/src/app/components/tables/flows-table-REST.tsx
Outdated
Show resolved
Hide resolved
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
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.
Checks have passed and this pull request is ready for manual review
I just want this code to run on unmount and not everytime handleAbortController() re-renders
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.
Checks have passed and this pull request is ready for manual review
Description
FTS Admin Rewrite (FTS Admin v2) first pull request for Flow table, Pending Flow Table, Organization Table and CRUD page, Keyword page and CRUD functionalities.
Features
hpc-ui
And a few more, for further readability please follow to the commit history.
Next steps