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

UI: Moves fetchdata() to the created() #4811

Merged

Conversation

utchoang
Copy link

@utchoang utchoang commented Mar 15, 2021

Description

This PR to move fetchData() to created() insteadof mount()

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@utchoang utchoang mentioned this pull request Mar 15, 2021
12 tasks
@utchoang utchoang marked this pull request as ready for review March 15, 2021 06:09
@utchoang
Copy link
Author

@blueorangutan ui

@blueorangutan
Copy link

@utchoang a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. [S]

@shwstppr
Copy link
Contributor

@utchoang will this offer any performance improvement? I hope we are not accessing/manipulating DOM elements in any of the fetchData methods

@utchoang
Copy link
Author

@shwstppr Yes. In apache/cloudstack-primate#719 then move fetchData to created from mounted to reduce wait time for API calls since the data has been created but the page has not yet been rendered

@rohityadavcloud
Copy link
Member

@utchoang what are the pros and cons of using one vs the other?

@utchoang
Copy link
Author

@rhtyd When the component is initialized, fetching data from the backend at created() will be done without having to wait for the DOM to be mounted or rendered as if it were called on mounted().

@davidjumani
Copy link
Contributor

@blueorangutan ui

@blueorangutan
Copy link

@davidjumani a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4811 (SL-JID-63)

Copy link
Contributor

@davidjumani davidjumani left a comment

Choose a reason for hiding this comment

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

LGTM. Did a quick round of testing
Forms appear to load a bit sooner

@rohityadavcloud
Copy link
Member

@utchoang can you fix the conflict against latest 4.15 branch

@rohityadavcloud
Copy link
Member

@utchoang can you fix the conflict?

@rohityadavcloud
Copy link
Member

@blueorangutan ui

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

Copy link
Member

@rohityadavcloud rohityadavcloud left a comment

Choose a reason for hiding this comment

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

LGTM proxing David's testing/review

@rohityadavcloud rohityadavcloud merged commit 4dcd9a2 into apache:4.15 Apr 5, 2021
@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4811 (SL-JID-88)

@utchoang utchoang deleted the feature/move-fetchdata-created branch April 5, 2021 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants