FEAT: Optimize List View Components with Table Design and Sorting Across Project Pages #819
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… added sorting functionality
tahierhussain
requested review from
Deepak-Kesavan,
athul-rs,
jagadeeswaran-zipstack,
kirtimanmishrazipstack and
vishnuszipstack
vishnuszipstack
requested changes
Oct 25, 2024
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx
Outdated
Show resolved
Hide resolved
frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx
Outdated
Show resolved
Hide resolved
Deepak-Kesavan
requested changes
Oct 26, 2024
|
|
jagadeeswaran-zipstack
requested changes
Nov 5, 2024
Comment on lines
10
to
26
| function ViewTools({ | ||
| isLoading, | ||
| isEmpty, | ||
| listOfTools, | ||
| setOpenAddTool, | ||
| listOfTools = [], | ||
| setOpenAddTool = () => {}, | ||
| handleEdit, | ||
| handleDelete, | ||
| titleProp, | ||
| descriptionProp, | ||
| iconProp, | ||
| descriptionProp = "", | ||
| iconProp = "", | ||
| idProp, | ||
| centered, | ||
| centered = false, | ||
| isClickable = true, | ||
| handleShare, | ||
| showOwner, | ||
| type, | ||
| handleShare = null, | ||
| showOwner = false, | ||
| type = "", | ||
| }) { |
There was a problem hiding this comment.
NIT: Move props with default values to bottom for better code readability.
Comment on lines
+129
to
+130
| id: user.id, | ||
| email: user.email, |
There was a problem hiding this comment.
Why optional chaining got removed here?
| (adapter, isEdit) => { | ||
| const requestOptions = { | ||
| method: "GET", | ||
| url: `${adapterBaseUrl}/users/${adapter.id}/`, |
There was a problem hiding this comment.
Please add optional chaining where ever necessary.
| ); | ||
|
|
||
| const renderDateColumn = useCallback( | ||
| (text) => (text ? moment(text).format("YYYY-MM-DD HH:mm:ss") : "-"), |
There was a problem hiding this comment.
Can we move the data formatter to static func? Are they reuseable?
| const { setPostHogCustomEvent } = usePostHogEvents(); | ||
|
|
||
| const { setAlertDetails } = useAlertStore(); | ||
| const axiosPrivate = useAxiosPrivate(); | ||
| const getBaseUrl = (sessionDetails) => |
There was a problem hiding this comment.
getBaseUrl naming is bit miss leading since we provide more than baseUrl also can we move this function to static functions where we actually provide base url (till org Id) when we pass sessionDetails?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
This PR optimizes and enhances the list view components across multiple pages (workflow, prompt studio, LLM, vector DB, embedding, and x2text). The improvements include transitioning to a table-based design and implementing a sorting feature.
Why
Enhancing the list view components was necessary to:
Name,Owner By,Created At, andModified At.How
Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
Yes, there are some substantial refactorings of the existing components in this PR, so there is a possibility of breaking existing features.
Database Migrations
NA
Env Config
NA
Relevant Docs
NA
Related Issues or PRs
NA
Dependencies Versions
NA
Notes on Testing
NA
Screenshots
Workflow Projects Page:
Prompt Studio Projects Page:
LLM Adapters Page:
VectorDB Adapters Page:
Embedding Adapters Page:
X2Text Adapters Page:
Checklist
I have read and understood the Contribution Guidelines.