Skip to content
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

Improve catalog properties modal tables #55

Merged
merged 7 commits into from
Aug 30, 2023

Conversation

mkralik3
Copy link
Contributor

@mkralik3 mkralik3 commented Aug 25, 2023

  • Tabs in properties detail modal
  • Add headers and api properties for camel components
  • Add filter to distinguish between path and query property
  • Show more tables in one modal page
  • Refactor/generalize filter to use them in all functions (all properties in getTableFromProperties functions can be filtered by IPropertiesTableFilter = { filterKey: string; filterValue: string }
    e.g. get only properties which have kind="path", pass filter like this into function
{
  filterKey: 'kind',
  filterValue: 'path',
}

@mkralik3 mkralik3 linked an issue Aug 25, 2023 that may be closed by this pull request
@mkralik3
Copy link
Contributor Author

mkralik3 commented Aug 25, 2023

Camel components:
Component properties tab:
image
Properties tab with two tables according to kind
image
Headers
image
API*[1]
image
No prop in kamelets
image

[1] - More information about API's are out of scope of this PR, API parameters have more methods with more properties, so the expendable rows with sub-rows needs to be implemented, at least with 3 level (apis->methods->methodParameters) ,, see AS2 or Twilio component => #60

@mkralik3 mkralik3 changed the title Improve catalog properties table Improve catalog properties modal tables Aug 25, 2023
@mkralik3 mkralik3 force-pushed the catalog_modal branch 5 times, most recently from 694d527 to 8f788e3 Compare August 28, 2023 15:12
@mkralik3 mkralik3 force-pushed the catalog_modal branch 2 times, most recently from 72c9e9a to d6dc804 Compare August 29, 2023 08:49
@mkralik3 mkralik3 marked this pull request as ready for review August 29, 2023 08:56
@mkralik3 mkralik3 requested a review from a team August 29, 2023 08:56
@lhein
Copy link
Contributor

lhein commented Aug 29, 2023

Can we put some blank space between the description of the component and the tabs for the properties tables?

@mkralik3
Copy link
Contributor Author

@lhein sure, I am currently working to add there also Enum and Autowired info

@mkralik3
Copy link
Contributor Author

mkralik3 commented Aug 29, 2023

done,
image

the Doc page: https://camel.apache.org/components/3.21.x/aws2-ddbstream-component.html#_component_options

@mkralik3
Copy link
Contributor Author

mkralik3 commented Aug 29, 2023

hm I don't understand why GitHub shows This branch has conflicts that must be resolved , since this branch is up to date and rebased with main
image
and when i create a new PR, GitHub is able to merge it automatically 🤷
image

Maybe it will be gone after next rebase with main.
But it doesn't matter. Once this PR will be approved, I will created another one [if the github still refuse merge this one] or merge it manually.

packages/ui/src/camel-utils/camel-to-table.adapter.ts Outdated Show resolved Hide resolved
export const PropertiesModal: FunctionComponent<IPropertiesModalProps> = (props) => {
let table: IPropertiesTable;
let tabs: IPropertiesTab[] = [];
switch (props.tile.type) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing here is that it will make the calculations on every render. It would be more convenient to store the tabs object using a useState hook and then update it using a useEffect that depends on props.tile.type.

The idea would be more or less something like this:

const [tabs, setTabs] = useState<IPropertiesTab[]>([]);

useEffect(() => {
  //  switch logic goes here

  // store the resulting calculation like
  setTabs(CALCULATED_TABS_HERE);
}, [props.tile.type]);

Also, is worth mentioning that unless we plan to reuse the returning array, it would be more convenient (IMHO) to return an array from the function, rather than providing one from the outside.

let tabs: IPropertiesTab[] = [];
transformCamelComponentIntoTab(tabs, props.tile.rawObject as ICamelComponentDefinition);

// vs 

const tabs = transformCamelComponentIntoTab(props.tile.rawObject as ICamelComponentDefinition);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tabs are in a different component, there is no re-render in the modal component However, of course, it can be some in the future so I will move it into useEffect() .

Copy link
Contributor Author

@mkralik3 mkralik3 Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lordrip Isn't there better to use useCallback / useMemo hooks in this case?
Because useEffect is running only after the component is rendered, so, for the first time, the empty table is shown for a few milliseconds (because the table is null for the first rendering). [Also with opening a different camel component, for milliseconds, the table from the previous component is shown ]

In other words, in this case, the computation is needed before the first rendering.
e.g.
https://github.com/KaotoIO/kaoto-next/blob/0fa042ad24b9cd08126d25255e76b682ffbaf238/packages/ui/src/components/PropertiesModal/PropertiesModal.tsx#L20-L38

I have tried to re-render the modal component (via useState and button) and the switch computation is not running before the modal is open again ( with a different camel component ) so it is working correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, let's go with your approach then 💪

@mkralik3 mkralik3 merged commit 9438ee0 into KaotoIO:main Aug 30, 2023
1 check passed
@lhein lhein added this to the 0.1.0 - Smart Connectors' MVP milestone Sep 21, 2023
@lhein lhein modified the milestones: Smart Connectors' MVP, Feature Parity Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Show the component properties rather than endpoint properties
3 participants