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

Feat: tree details page #96

Merged
merged 8 commits into from
Jul 17, 2024
Merged

Feat: tree details page #96

merged 8 commits into from
Jul 17, 2024

Conversation

mari1912
Copy link
Collaborator

@mari1912 mari1912 commented Jul 16, 2024

Description

  • Add tree navigation ui
  • Add navigation
  • Add api call to tree

Related Issues

How to test it

  • Run docker compose

Visual reference

image

@mari1912 mari1912 self-assigned this Jul 16, 2024
@mari1912 mari1912 force-pushed the feaat/tree-details-page branch 2 times, most recently from 692744c to 5058619 Compare July 16, 2024 23:34
@mari1912 mari1912 requested a review from lfjnascimento July 16, 2024 23:35
@mari1912 mari1912 changed the title Feaat: tree details page Feat: tree details page Jul 16, 2024
@mari1912 mari1912 marked this pull request as ready for review July 16, 2024 23:36
@lfjnascimento
Copy link
Contributor

lfjnascimento commented Jul 17, 2024

Nit: There is no need to use the word Component in the name of a component

dashboard/src/routes/TreeDetails/TreeDetails.tsx Outdated Show resolved Hide resolved
<div className="flex w-2/3 px-6 items-center">
{/* placeholder for search */}
{/* TODO: use i18n for the input placeholder */}
<Input
Copy link
Contributor

Choose a reason for hiding this comment

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

If you can, it would be good to create a DebounInput component that accepts the interval

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure if I get your point, the DEBOUNCE_INTERVAL is being used in the const debouncedInput that calls useDebounce. The only function of the input here is to get the input value. How do you though in doing it?

Copy link
Contributor

Choose a reason for hiding this comment

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

My ideia is a component like <DebounceInput onChange={...} interval={...} /> that holds the debounce logic (useDebounce) and calls onChange only when needed respecting the debounce interval.

Copy link
Collaborator Author

@mari1912 mari1912 Jul 17, 2024

Choose a reason for hiding this comment

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

and how would you make to get the value from the input to filter the page by it? also, we have to save all the data that the user type, just the filter value changes that will be updated later

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case we would use as

<DebounceInput
    onChange={onInputSearchTextChange}
    interval={debouncedInput}
  />

The onInputSearchTextChange will be called respecting the debounce. That way we will not re-render the page on every key stroke just the DebounceInput component.
The way it is know we are re-render the Trees for every stroke (because it update the inputSearchText state) and also for every debounce trigger (by updating debouncedInput state)

dashboard/src/components/Table/TreeTable.tsx Outdated Show resolved Hide resolved
dashboard/src/components/Button/FilterButton.tsx Outdated Show resolved Hide resolved
dashboard/src/components/Tabs/TreeDetailsTab.tsx Outdated Show resolved Hide resolved
dashboard/src/routes/TreeDetails/TreeDetails.tsx Outdated Show resolved Hide resolved
return res.data;
};

export const useTreeDetail = (treeId: string): UseQueryResult => {
Copy link
Contributor

@lfjnascimento lfjnascimento Jul 17, 2024

Choose a reason for hiding this comment

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

Suggested change
export const useTreeDetail = (treeId: string): UseQueryResult => {
export const useTreeDetails = (treeId: string): UseQueryResult<TreeDetails> => {

so we can avoid data as TreeDetailsType such as here

@lfjnascimento
Copy link
Contributor

The summary card content in scrolling on top of TopBar
image

@lfjnascimento
Copy link
Contributor

Left menu is not following the page scroll

@mari1912 mari1912 force-pushed the feaat/tree-details-page branch from 5058619 to 3d1aa1a Compare July 17, 2024 14:46
@mari1912 mari1912 requested a review from lfjnascimento July 17, 2024 14:46
mari1912 added 2 commits July 17, 2024 11:54
- add a router
- remove app.css and app.tsx and use main.tsx instead
- make a basic layout as a root
- add a trees router
@mari1912 mari1912 force-pushed the feaat/tree-details-page branch from 3d1aa1a to 7cfc8c8 Compare July 17, 2024 14:54
@mari1912 mari1912 force-pushed the feaat/tree-details-page branch from 7cfc8c8 to ba924a8 Compare July 17, 2024 16:32
@mari1912 mari1912 force-pushed the feaat/tree-details-page branch from f47555a to 4f97793 Compare July 17, 2024 17:46
@mari1912 mari1912 merged commit 39e20eb into main Jul 17, 2024
4 checks passed
@mari1912 mari1912 deleted the feaat/tree-details-page branch July 17, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Tree Details Page
2 participants