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

Jordan/1603 sidebar links #1633

Merged
merged 12 commits into from
Nov 23, 2018
Merged

Jordan/1603 sidebar links #1633

merged 12 commits into from
Nov 23, 2018

Conversation

jbibla
Copy link
Collaborator

@jbibla jbibla commented Nov 22, 2018

Closes #1603

Description:

  • fixed sidebar bug
  • fixed some other errors
  • made some parts of the app more consistent / minor style UPGRADEZZ

❤️ Thank you!


  • Added entries in CHANGELOG.md with issue # and GitHub username
  • Reviewed Files changed in the github PR explorer

@codecov
Copy link

codecov bot commented Nov 22, 2018

Codecov Report

Merging #1633 into develop will decrease coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #1633      +/-   ##
===========================================
- Coverage    97.63%   97.63%   -0.01%     
===========================================
  Files          101      101              
  Lines         2031     2029       -2     
  Branches        90       90              
===========================================
- Hits          1983     1981       -2     
  Misses          37       37              
  Partials        11       11
Impacted Files Coverage Δ
...pp/src/renderer/components/staking/LiValidator.vue 96% <ø> (ø) ⬆️
.../renderer/components/governance/TableProposals.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/wallet/LiCoin.vue 100% <ø> (ø) ⬆️
...rc/renderer/components/staking/TableValidators.vue 97.22% <ø> (ø) ⬆️
app/src/renderer/components/wallet/PageWallet.vue 91.42% <ø> (-0.24%) ⬇️
app/src/renderer/components/common/AppMenu.vue 100% <ø> (ø) ⬆️
.../src/renderer/components/governance/LiProposal.vue 100% <ø> (ø) ⬆️
...c/renderer/components/staking/TabMyDelegations.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/common/AppHeader.vue 93.75% <ø> (ø) ⬆️
app/src/renderer/routes.js 100% <ø> (ø) ⬆️
... and 1 more

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Changes LGTM. Will test it shortly

@@ -16,7 +12,7 @@
|
router-link(:to="{name: 'Validators'}") the validator list
|
| to spread some of your Atoms around.
Copy link
Contributor

Choose a reason for hiding this comment

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

lol why did we had spread

@@ -26,8 +31,7 @@ export default {
if (this.proposal.proposal_status === `Passed`)
return {
button: null,
message: `This proposal has passed`,
color: `green`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should leave it green here

Copy link
Collaborator Author

@jbibla jbibla Nov 23, 2018

Choose a reason for hiding this comment

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

it's not a little green dot anymore - it's a green checkmark.

if felt the green status dot should be for signalling "voting".

truthfully, i'd like to move status from the little dot into its own column / tab soon.

@@ -45,7 +49,7 @@ export default {
return {
button: `vote`,
message: `Voting for this proposal is open`,
color: `blue`
color: `green`
Copy link
Contributor

Choose a reason for hiding this comment

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

and use orange or similar color here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no green is good for open voting! like a traffic light! GO GO GO!

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

tested it. Please fix all the warnings on the console. E.g:

A warning has occurred: Property or method "totalRewards" is not defined on the instance but referenced during render...

@fedekunze
Copy link
Contributor

PageWallet's style looks more consistent now with the rest of the app. I still have a feeling that it's missing something... maybe a header like the image from #1424 or something similar from the mobile app ideas I pinned on Slack. Not for the scope of this PR though

@jbibla
Copy link
Collaborator Author

jbibla commented Nov 23, 2018

PageWallet's style looks more consistent now with the rest of the app. I still have a feeling that it's missing something...

totally agree. but trying to stay focussed on our milestones 😄

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

tested ACK

@fedekunze fedekunze merged commit 9d8e369 into develop Nov 23, 2018
@fedekunze fedekunze deleted the jordan/1603-sidebar-links branch November 23, 2018 14:42
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.

staking and governance do not appear selected in the sidebar
2 participants