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

loader added on encrypt wallet, create wallet, restore wallet and few… #206

Merged
merged 6 commits into from
Nov 3, 2017

Conversation

vikas-cis-zz
Copy link

… design tweaks

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 53.105% when pulling 1f47c2e on vikas-cis:material_bug_fixing into f70fe36 on particl:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 53.105% when pulling d056d40 on vikas-cis:material_bug_fixing into f70fe36 on particl:develop.

@kewde
Copy link
Collaborator

kewde commented Oct 26, 2017

The RPC state service should not be used for things like this.

kewde
kewde previously requested changes Oct 26, 2017
Copy link
Collaborator

@kewde kewde left a comment

Choose a reason for hiding this comment

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

Do not use state service.

@rynomster
Copy link
Member

I don't see what's wrong with using the state service for this? It is global state

@kewde
Copy link
Collaborator

kewde commented Oct 27, 2017

The state service flattens everything, we have no clue which variables are stored in it and under which names. I don't think it's very smart to use the same state service for our own variables, it will eventually become one big mess where nobody knows what's what.

At the very least tag the variable names with something that ensures it doesn't collide with critical variables (such as those returned by the rpc).

A better approach imo would be to get rid of the flattening and use a different state for UI/global stuff.

@rynomster
Copy link
Member

okay, @vikas-cis please prefix it with ui: or something obvious, so that we know its not related to application state but only to do with UI state.. At a later stage we can compartmentalise the states, but I don't see it as an immediate requirement.

@vikas-cis-zz
Copy link
Author

Sure its done :)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 53.105% when pulling b487a92 on vikas-cis:material_bug_fixing into f70fe36 on particl:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-5.0%) to 48.219% when pulling 679e1da on vikas-cis:material_bug_fixing into a81acbe on particl:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 53.055% when pulling a59bb35 on vikas-cis:material_bug_fixing into a81acbe on particl:develop.

Copy link
Member

@rynomster rynomster left a comment

Choose a reason for hiding this comment

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

Looks very nice! Thanks @vikas-cis :D

@rynomster rynomster dismissed kewde’s stale review November 3, 2017 15:40

Prefixed with 'ui:' so that it's clear that its UI related only

@rynomster rynomster merged commit 144de31 into particl:develop Nov 3, 2017
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.

5 participants