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

Config file handling #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

YonatanKra
Copy link
Contributor

Config handling should be async only on load (or on refresh). For this
I've made the following changes/additions:

  1. The graph is not drawn on load - only when done loading the config
    file (or on failure).
  2. The config is requested in the Factory's constructor - if request
    fails, asks for defaultConfig.
  3. Added local method defaultConfig.
  4. Added prototype method: getConfig
  5. initLayout uses existing config (should explicitely use
    "refreshConfig" in order to change the
  6. Added prototype method: "refreshConfig" which fetches config from the
    config file.

Config handling should be async only on load (or on refresh). For this
I've made the following changes/additions:
1) The graph is not drawn on load - only when done loading the config
file (or on failure).
2) The config is requested in the Factory's constructor - if request
fails, asks for defaultConfig.
3) Added local method defaultConfig.
4) Added prototype method: getConfig
5) initLayout uses existing config (should explicitely use
"refreshConfig" in order to change the
6) Added prototype method: "refreshConfig" which fetches config from the
config file.
There was an error when accessing the Ajax response. Was trying to get
config directly from "response" but should have been "response.data"
@AmitMY
Copy link
Contributor

AmitMY commented Sep 19, 2017

In the latest version, you can set config from the component, or if not setting any using a default config (eliminating the config file)
You can also change the config whenever and it should rerender.

Is there anything else in this PR still relevant?

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