Fix issue with Table Chart #531
Conversation
|
If I try to add the same table twice, using the front-end, it looks like this: https://i.imgur.com/JRNNvmC.png I suspect it might be something to do with the ID being duplicated or something. This is on chicken after adding this PR to it. The 'load more' issue also still happens, but that was for another ticket right? |
|
Can you do it on sandbox? As I wasn't able to reproduce this on my end and multiple table charts work just fine. @DanyCaissy |
|
@HardeepAsrani Yes, the previous image is on sandbox. http://cute-chicken.w6.wpsandbox.pro/wp-admin/post.php?post=14&action=edit |
|
Ahaa, got it. Thanks. I'll get it fixed. :) |
This makes sure all blocks have different IDs so they don't conflict.
This will exclue ChartJS from Rest route. Fixes #532 (comment)
|
@DanyCaissy This should fix both issues, the Table chart appearing twice & this one: #532 (comment) Let me know if it works. |
|
@HardeepAsrani in the future it would be best if you could add @DanyCaissy as a reviewer for the PR so that I can merge only when he approves the PR. |
contactashish13
added
the
bug
|
@HardeepAsrani I've added your dev branch to cute-chicken and the same issues still occur for me. This page: http://cute-chicken.w6.wpsandbox.pro/wp-admin/post.php?post=14&action=edit |
|
Just to be sure, I did: git clone https://github.com/HardeepAsrani/visualizer |
|
I rebased with the latest 3.3.1 branch and I get this after setting up the plugin. Is there an issue with the plugin? @contactashish13
|
|
@HardeepAsrani had that been a regular problem @DanyCaissy would have faced it too. I've tested the 3.3.1 branch with and without pro and I don't see this error. Please check if its not a problem at your end. |
|
I tried again and it happened again to me. It also happened with the Codeinwp repo. :( |
|
@HardeepAsrani can you share a fresh barebones public instance where I can troubleshoot this? |
|
@DanyCaissy Were you able to reproduce the double table issue again? It's working just fine. Might have been cache. :) |
|
@HardeepAsrani On your version now it wont show me tables at all to add: https://i.imgur.com/ZdQVqWs.png I also don't see all of the charts http://cute-chicken.w6.wpsandbox.pro/wp-admin/post.php?post=70&action=edit |
|
This was in another browser so no cache issue. |
|
There are two issues:
I guess you are not able to reproduce on your side because of the first issue. Every chart has different meta, depending on when they were made. You should delete all other charts but table to reproduce and see if it works. Meanwhile, I'll solve the first issue. |
|
@DanyCaissy I've fixed the issue on that Sandbox site. Can you see if it works now? |
|
Heya @HardeepAsrani I wasn't able to reproduce the issue where some tables were invisible, so maybe that was fixed successfully. There still seems to be something wrong with the pagination or count, right now it looks like this: https://i.imgur.com/l2n5qh2.png However, if I click load more, nothing happens (there is not 7th chart to display, the 7th is chartjs). The desired behaviour would be that if there is 6 charts, there should be no 'load more' button. |
|
Yea, that's a known issue, unfortunately. We can't know if there any charts left to load unless we load all of them at once. We can load the total count but since we're filtering out ChartsJS charts, it makes it bit complex. :( |
|
Well, if that's the only issue it causes, people can live with it. |
This fixes the issue which we have been having for weeks is now fixed. The table chart should work fine. To make sure it's fixed, @DanyCaissy can try to update with this on http://cute-chicken.w6.wpsandbox.pro/ to make sure it works.
Fixes #506 & #526
cc @contactashish13