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

fix: show the market chart when enabling it on the profile #1219

Merged
merged 1 commit into from May 6, 2019

Conversation

@j-a-m-l
Copy link
Contributor

commented May 2, 2019

Proposed changes

The market chart wasn't properly enabled or disabled when the related setting was changed on the profile edition.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes
@codecov-io

This comment has been minimized.

Copy link

commented May 2, 2019

Codecov Report

Merging #1219 into next will decrease coverage by 0.14%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #1219      +/-   ##
==========================================
- Coverage   39.28%   39.14%   -0.15%     
==========================================
  Files         223      223              
  Lines        5915     5916       +1     
  Branches     1176     1179       +3     
==========================================
- Hits         2324     2316       -8     
- Misses       3393     3401       +8     
- Partials      198      199       +1
Impacted Files Coverage Δ
src/renderer/components/utils/LineChart.js 10% <0%> (ø) ⬆️
src/renderer/pages/Dashboard.vue 17.39% <25%> (-2.61%) ⬇️
...rc/renderer/components/MarketChart/MarketChart.vue 22.47% <50%> (-8.3%) ⬇️
src/renderer/components/Network/NetworkModal.vue 28.85% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39438a5...410bab9. Read the comment docs.

@alexbarnsley

This comment has been minimized.

Copy link
Member

commented May 3, 2019

What exactly is the issue here? I can enable and disable the chart on the current next branch, and the setting saves between reloads for my mainnet profile. Can you explain the steps to recreate the issue this is fixing please?

Edit: or is this related to the option in the settings panel on the sidebar?
Edit 2: I can't see this option on the next branch. I saw it on #1180 so was unsure

@j-a-m-l

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

@alexbarnsley the menu to enable and disable the market chart was moved to the profile edition. On next it should not exist on the sidebar menu.

The problem occurs when you edit your profile to enable the chart, but then, the chart area is visible on the dashboard, but it's empty. It's related to how the chart.js library works: apertureless/vue-chartjs#372

@alexbarnsley alexbarnsley merged commit 488a5ec into next May 6, 2019

1 check passed

ci/circleci: test-node-11 Your tests passed on CircleCI!
Details

@ArkEcosystemBot ArkEcosystemBot deleted the fix-enabling-market-chart branch May 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.