Skip to content
This repository has been archived by the owner on Dec 13, 2020. It is now read-only.

Optimizations to rendering and architecture (2nd try) #2519

Merged
merged 10 commits into from
Jan 14, 2020
Merged

Conversation

siemiatj
Copy link
Contributor

@siemiatj siemiatj commented Jan 8, 2020

Related to #2473

I'm not happy with this solution, as it does add some noise and additional code, but because some components were built this way and our data structures are like that - we can't really avoid that. Hopefully some of this will go away once we can switch tables to use redux.

@petrican
Copy link

@siemiatj . Build fails in jenkins and it has a conflict. Pls fix. Thank you

Copy link

@petrican petrican left a comment

Choose a reason for hiding this comment

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

Pls fix the conflict file. Thanks

Copy link

@petrican petrican left a comment

Choose a reason for hiding this comment

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

Pls see my comments. This brings some performance boost but doesn't fix all the issues with webUI performance. Thanks

src/components/widget/Lookup/Lookup.js Show resolved Hide resolved
src/components/widget/Lookup/RawLookup.js Outdated Show resolved Hide resolved
src/components/widget/SelectionDropdown.js Show resolved Hide resolved
@petrican
Copy link

petrican commented Jan 10, 2020

@siemiatj - I also spot that this crashes the UI. If you go to the URI from the snapshot and click on the three dots it will trigger those errs in console and UI becomes unresponsive
Screenshot 2020-01-10 at 17 22 06

@siemiatj
Copy link
Contributor Author

siemiatj commented Jan 10, 2020

@petrican what URI from the snapshot ? The alert suggests that you're leaving the page without saving, so as long as it's visible - the page will be unresponsive.

@petrican
Copy link

@petrican what URI from the snapshot ? The alert suggests that you're leaving the page without saving, so as long as it's visible - the page will be unresponsive.

http://localhost:3000/window/181/1001579 . When you are on that page and click on the three dots on the top left you get that err

#2473 remove some anonymous functions from Table components
#2473 upgrade transitions library
#2473 rewrite loader to a stateless component
#2473 switch RawWidget to PureComponent
#2473 change bunch of components to use PureComponent
#2473 move more components to using PureComponent
#2473 remove anonymous functions for WidgetTooltip
#2473 disconnect Lookup from the store and extend PureComponent
#2473 Lookup cleanup
#2473 simplify RawLookup and remove anonymous functions
#2473 remove anonymous functions from Header and switch to `getWidgetData`
#2473 rewrite MasterWidget to properly use `getWidgetData` instead of `widgetData` object
#2473 optimize header and inbox
#2473 simplify data structures to widgets and remove unused props
#2473 move Table to PureComponent
#2473 create selector for MasterWindow data
#2473 remove anonymous event handlers from DocumentListContextShortcuts
#2473 switch Prompt to PureComponent
#2473 revert `why-did-you-update` to stable for master
#2473 add missing props
@siemiatj
Copy link
Contributor Author

@petrican I can't reproduce anything like that, but I noticed the icon was not being shown so fixed this instead.

https://recordit.co/26bPZ0nPRN

@siemiatj siemiatj requested review from petrican and removed request for petrican January 13, 2020 13:54
Copy link

@petrican petrican left a comment

Choose a reason for hiding this comment

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

I am not 100% in favor of the solution with the PureComponents but I am willing to make this compromise for a performance boost. Let's see how this goes. In the near future we might refactor to use functional components.

@siemiatj
Copy link
Contributor Author

Yeah, I'd go with functional components in many places but with the current architecture we must boost performance whenever possible.

@siemiatj siemiatj merged commit 1acd2f5 into master Jan 14, 2020
@siemiatj siemiatj deleted the dev-2473-master branch January 14, 2020 11:30
@siemiatj siemiatj restored the dev-2473-master branch January 16, 2020 08:47
@teosarca teosarca deleted the dev-2473-master branch February 11, 2020 09:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants