fossology / FOSSologyUI

Repository to hold the new UI framework for FOSSology built with React
https://fossology.github.io/FOSSologyUI/
GNU General Public License v2.0
49 stars 88 forks source link

Added lazy loading components #166

Closed soham4abc closed 2 years ago

soham4abc commented 2 years ago

Signed-off-by: Soham Banerjee sohambanerjee4abc@hotmail.com

Description

Added Lazy loading imports to the Routes.jsx file

Changes

Files changed : src/Routes.jsx

Fixes #160

soham4abc commented 2 years ago

Formatted the file as per requirement! PTAL @GMishx

soham4abc commented 2 years ago

I am working on the error give me 10 mins

soham4abc commented 2 years ago

image Fixed! PTAL @GMishx

soham4abc commented 2 years ago

If the test fails again pls do let me know what exact changes I need to do

GMishx commented 2 years ago

@Aman-Codes @Shruti3004 @sjha2048 can you please help with the test cases.

Because of lazy loading, the snapshot is loaded as just

<div>
  Loading... 
</div>
Aman-Codes commented 2 years ago

@soham4abc You had to update the test to take snapshot according to lazy loading. Now, I have added that myself in this PR itself.

So @GMishx its good to merge now

soham4abc commented 2 years ago

@soham4abc You had to update the test to take snapshot according to lazy loading. Now, I have added that myself in this PR itself.

So @GMishx its good to merge now

Thank you for your help!

GMishx commented 2 years ago

@soham4abc , can you please squash your commits and format them according to contributing guidelines?

You can do so by rebasing the branch with main and preserve the commit from Aman

soham4abc commented 2 years ago

@soham4abc , can you please squash your commits and format them according to contributing guidelines?

You can do so by rebasing the branch with main and preserve the commit from Aman

PTAL

GMishx commented 2 years ago

Oh, there is one additional commit in this PR now. I think it is 546f10f6a9ba7b8cdec867f11b4d4746005bf385

soham4abc commented 2 years ago

Oh, there is one additional commit in this PR now. I think it is 546f10f

Should it stay or should I remove it?.. It was fetched from the rebase

github-actions[bot] commented 2 years ago

This pull request has conflicts, please rebase to resolve those before we can evaluate the pull request.

Shruti3004 commented 2 years ago

Hey @soham4abc how did you rebased your branch, you are missing something!

soham4abc commented 2 years ago

Hey @soham4abc how did you rebased your branch, you are missing something!

I am on it I messed up... will fix this asap

Shruti3004 commented 2 years ago

Hey @soham4abc how did you rebased your branch, you are missing something!

I am on it I messed up... will fix this asap

No worries, it happens. Take your time. Though if something major happened you can use git reflog command to reverse it back.

soham4abc commented 2 years ago

Hey @soham4abc how did you rebased your branch, you are missing something!

I am on it I messed up... will fix this asap

No worries, it happens. Take your time. Though if something major happened you can use git reflog command to reverse it back.

All issues fixed and commit history cleaned and commits squashed! Please do have a look and let me know if any more changes are required or not! @GMishx @Shruti3004

GMishx commented 2 years ago

Hey @soham4abc , looks like something got messed-up in the Routes.jsx file while rebasing. There are two imports for each page now.

Please fix and amend the commit

soham4abc commented 2 years ago

Hey @soham4abc , looks like something got messed-up in the Routes.jsx file while rebasing. There are two imports for each page now.

Please fix and amend the commit

I discovered it.. on it give me 10 minutes

soham4abc commented 2 years ago

@GMishx can you please re run the tests.

GMishx commented 2 years ago

I am getting following warnings whenever I load a page. Is it related to this PR?

Warning: Failed prop type: Invalid prop `component` supplied to `PrivateLayout`.
PrivateLayout@http://localhost:3000/static/js/57.chunk.js:56:23
C@http://localhost:3000/static/js/vendors~main.chunk.js:36038:31
Switch@http://localhost:3000/static/js/vendors~main.chunk.js:35983:29
Router@http://localhost:3000/static/js/vendors~main.chunk.js:35416:30
BrowserRouter@http://localhost:3000/static/js/vendors~main.chunk.js:35036:35
Routes
Ge@http://localhost:3000/static/js/vendors~main.chunk.js:42047:67
GlobalProvider@http://localhost:3000/static/js/main.chunk.js:1167:24
App@http://localhost:3000/static/js/main.chunk.js:111:63
Suspense index.js:1
Warning: Failed prop type: Invalid prop `component` supplied to `AdminLayout`.
AdminLayout@http://localhost:3000/static/js/56.chunk.js:56:21
C@http://localhost:3000/static/js/vendors~main.chunk.js:36038:31
Switch@http://localhost:3000/static/js/vendors~main.chunk.js:35983:29
Router@http://localhost:3000/static/js/vendors~main.chunk.js:35416:30
BrowserRouter@http://localhost:3000/static/js/vendors~main.chunk.js:35036:35
Routes
Ge@http://localhost:3000/static/js/vendors~main.chunk.js:42047:67
GlobalProvider@http://localhost:3000/static/js/main.chunk.js:1167:24
App@http://localhost:3000/static/js/main.chunk.js:111:63
Suspense index.js:1
soham4abc commented 2 years ago

Failed prop type: Invalid prop component supplied to

https://stackoverflow.com/questions/33950433/warning-failed-proptype-invalid-prop-component-supplied-to-route?rq=1

This might help

GMishx commented 2 years ago

I am not good with JS. With my limited knowledge and from stackoverflow link, I understood that since the components are now lazy loaded, thus while adding them to Routes they are not available??

Because all the components use export default Component syntax rather than module.exports = Component as suggested in answers.

@Aman-Codes @Shruti3004 @sjha2048

Aman-Codes commented 2 years ago

https://github.com/fossology/FOSSologyUI/blob/a7386bb909b57dac2e1123bb51291aaa1c097e31/src/shared/PublicLayout.jsx#L42-L42

https://github.com/fossology/FOSSologyUI/blob/a7386bb909b57dac2e1123bb51291aaa1c097e31/src/shared/PrivateLayout.jsx#L55-L55

https://github.com/fossology/FOSSologyUI/blob/a7386bb909b57dac2e1123bb51291aaa1c097e31/src/shared/AdminLayout.jsx#L55-L55

Hi @soham4abc Could you please replace the above 3 line with the below line. It will fix the error faced by @GMishx

  component: PropTypes.oneOfType([PropTypes.element, PropTypes.elementType]),
soham4abc commented 2 years ago

The warnings are removed. Please continue testing @GMishx @Aman-Codes

soham4abc commented 2 years ago

Sorry... This PR should have been much smoother. Sorry to bother you all! And thanks for your help @GMishx @Aman-Codes @Shruti3004