Skip to content

ui/ performance optimization - #9

Merged
leon-up9 merged 6 commits into
developfrom
ui/performance-optimization
Jun 12, 2022
Merged

ui/ performance optimization#9
leon-up9 merged 6 commits into
developfrom
ui/performance-optimization

Conversation

@leon-up9

@leon-up9 leon-up9 commented Jun 7, 2022

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread ui/src/components/PathList/PathList.tsx Outdated
Comment thread ui/src/components/PathList/PathList.tsx Outdated
Comment thread ui/src/components/PathList/PathList.tsx Outdated
Comment thread ui/src/components/PathList/PathList.tsx Outdated
Comment thread ui/src/components/PathList/PathList.tsx Outdated
Comment thread ui/src/components/PathList/PathList.tsx Outdated
<PathListItem changeLogItem={change} showChangeType={type} /></div>)
}
{filteredChanges?.map((change: DataItem, index: number) =>
<PathListItem key={index} changeLogItem={change} showChangeType={type} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

it is not best practice to use index for react element key..
do we have maybe a key or some identifier for each dataItem that we can use instead?

@leon-up9 leon-up9 Jun 8, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i know...but we dont have identifier.
should i inject it once the data injected?

Comment thread ui/src/components/PathList/PathListItem.tsx Outdated
Comment thread ui/src/components/PathList/PathListItem.tsx Outdated
@@ -0,0 +1,24 @@
import { useState, useEffect } from "react";

const useDebounce = (value, delay) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why can't we use lodash debounce or something else that already exists?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

its the solution i found to seperate the state update and the filter operation.
I as see it the trade off will be to hold another variable and make the debounce on him and useEffect to listen to that change to make the filter operation.
I come from the assumption, the state must be updated immediately

Comment thread ui/src/components/PathList/PathDisplay.tsx Outdated

@lirazyehezkel lirazyehezkel left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

added some comments

@leon-up9
leon-up9 merged commit 380696a into develop Jun 12, 2022
@leon-up9
leon-up9 deleted the ui/performance-optimization branch June 12, 2022 09:09
leon-up9 added a commit that referenced this pull request Jun 12, 2022
* tooltip & width change (#7)

Co-authored-by: Leon <>

* Ui/group-changes-by-change-type (#8)

map changes

Co-authored-by: Leon <>

* ui/ performance optimization (#9)

* avoid unnecessery renderes

* removed redundant code

* added category change header

* cleaned redeundant

* pr comments

* put color

Co-authored-by: Leon <>

* ui/ restore collapse button (#12)

* avoid unnecessery renderes

* removed redundant code

* recover lost change type header

* added category change header

* cleaned redeundant

* pr comments

* put color

* split accordions state to  2 atoms

* atom rename

* spaces removed

* fix bad setState

* reverst change

* rel changed

* pr comments

Co-authored-by: Leon <>

Co-authored-by: Gustavo Massaneiro <gustavomassa123@gmail.com>
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.

2 participants