Implementing these changes resulted in a View displaying the first 110 rows of a 2,000,000 row CSV file taking 64.51 ms to render versus 21000 ms.
The issue at play here is the query-based nature of Views. Your parent View will be one query, and the Chart Attachment will be a different one. These can work together well if the categories are the same or if the Chart Attachment is a subset of the parent display. But when the child has different categories, there's not a good way to tell the View where these new values should fit. I think solving this might be too complicated.
I have two recommendations if you face this issue:
1) populate your data with null values until the categories match
2) use the Grouping feature in views, which might achieve what you're hoping (the drawback is that you will have to have the same chart type)
@mutasim al-shoura - thank you for creating this ticket. If you are using Highcharts, here are my recommendations that don't require a patch:
- Enable the Accessibility, Exporting, and Export-Data, and Texture libraries (you can enable them here: /admin/config/content/charts)
- Add the Highcharts Caption module*: https://www.drupal.org/project/charts_highcharts_caption →
*If you scroll to the bottom of https://www.highcharts.com/docs/accessibility/accessibility-module, you'll see that using the caption is a great option for the description, and the nice thing is that it gets included in the Chart export, too.
However, the above is really Highcharts-specific. Would you be interested in making a more generic solution, which would apply to all the libraries? You have a start to that in your patch.
By the way, if you could create a merge request, I would appreciate that over a patch, as it runs code checks and tests.
@bspeare - I'm not sure about with Google, but this is actually fixed for Highcharts in 5.1.x. If that's what you're using, navigate to /admin/config/content/charts, the select Highcharts Settings > Global Options > Language and then enter a comma in the "Number formatting: Thousand separator" field (and save the page). If this is for Google, you're welcome to open a new ticket.
Thank you, @nikathone! I like that you make it work with the original function. I'm happy with this and the code quality checks pass.
Thank you, @annemiekb - I just created a MR with your patch to help benefit from the automated tests and make it a little easier for folks to work on the issue.
andileco → made their first commit to this issue’s fork.
Please review for the sort feature. I think anything bigger than what I've done here will be too much to make a timely commit. Are you OK with renaming the ticket to focus on the sort feature? And then users who want other Views Agent related features can create new tickets?
Going to add a method/prompt to determine sorts in Views - that's currently missing.
This is fixed, as the module is compatible with ^10 and ^11.
This has been reviewed and tested and is ready to be released to users.
Uploading a video so that people can have an expectation of what the module should look like when working.
I have this where I want it according to my own testing. Would love some feedback.
Apologies for the delay. I created a MR but gave you credit. Thank you!
andileco → made their first commit to this issue’s fork.
Thank you, @stefan.korn - this is a lot of work!
I started making a few adjustments. Can you double-check the schema?
@erutan, yes, exactly, that's how they can opt-in other projects where they need a JS library to be installed.
@erutan - I've attached a screenshare. Please take a look and let me know what you are doing differently.
- Fix eslint
- Update C3 libraries.yml
- Update Highcharts in package.json
@erutan, good catch about solid-gauge.
I'm realizing that it may make sense to update the readme to be more specific to charts. The "workspaces" that you used travelled through *all* the modules in your site, and some of them must be using outdated npm versions. Instead, we should put, "web/modules/contrib/charts/**/*" for "workspaces".
That said, I did identify that some D3 packages need updating. I don't know if these are recent updates. But anyway, those will need to be updated before this is ready to go live.
OK, I figured out that the issue occurs when you have BigPipe enabled. I have it disabled in my test environment.
As I mentioned before, I did observe that the module (and particularly the JS override: the last Chart in How to Use the Charts API) is working with the weight removed. I'm creating a MR of your patch and will commit it today.
andileco → made their first commit to this issue’s fork.
Hi @lohndaniel. Thank you for posting the steps to reproduce and your patch, but I'm unable to reproduce the issue (see attached). Can you provide any more information about how you installed the libraries and if you have any custom JS in your site? To be fair, I'm also not experiencing an issue with your patch applied either. But before I commit, I'd like to better understand why you are getting an error and I'm not.
Thank you for your work on this, @tommasorandazzo! Would you be willing to take a look at the branch I created? This allows one to have a package.json at the root like:
{
"name": "foo",
"private": true,
"scripts": {
"postinstall": "npm run libraries:copy --workspaces --if-present"
},
"workspaces": [
"web/modules/***/**/*"
]
}
and when you run `npm install`, it puts the libraries into their proper folders (if you only wanted certain charts submodules, you could adjust the workspaces).
You are clearly more adept at using npm than me, so please let me know if your thoughts on this approach.
Hi! This is mostly working well, except in the case where the field plugin is used and the "Enable fields by default" is checked. In that case, the sort removes the selection (for checkboxes or multi-select), although the fields stay in place. Please see the attached video.
Hi all, I don't use Commerce myself, and so testing this always proves to be a big hassle. This ticket is also marked for 8.x-1.x, which is two versions back. @dahousecat is your patch for 3.0.x? If so, please update the ticket. If someone can provide some steps to repeat and test the patch, I would appreciate it.
Hi @chike! For our use-case, we do have some user roles who create and edit certain types of nodes, and it's a less disruptive user experience if they are not switched between an admin and front-end theme. I believe this patch will work with Claro, Gin, and Bootstrap, as it uses a more generic selector. So, it might be worth it for the maintainers to incorporate.
We don't have a convention for charts_library_type, but we do for charts_library_extendedfeature. Check out how Highcharts is handling additional files (JS modules) and the relatively new library+type method. We might not need a separate module.
andileco → created an issue.
Thank you! There's a minor issue where if you move from the third option to the second (on a taxonomy term field), then it looks different than if you move from the first to the second. But I'm going to make it a follow-up issue.
For some reason, GitLab wanted to apply my patch to the 8.x branch versus the 2.x branch. The MR that's showing now is the one to use.
andileco → changed the visibility of the branch 3543115-select-parents-automatically to hidden.
@yospyn, please try this MR. Make sure to clear your cache (including browser cache so the updated JS file loads). For me, this works on a Bootstrap Barrio-based subtheme and continues to work with Claro. Please let me know if the same goes for you.
The failed code quality tests shown in the MR pipelines are from the existing codebase and are not related to my patch.
andileco → made their first commit to this issue’s fork.
andileco → created an issue.
Thank you! This is working well.
Thank you, and sorry for the delay!
andileco → made their first commit to this issue’s fork.
Things seem to be working well - thank you!! One thing that I think needs to happen from a UX perspective is to provide a message if the user selects an option but the underlying data doesn't support it. For example, if there are no fields of a select type, then we should have a message that no eligible fields were found. For example, all three options show up for me for a view based on Views CSV Source.
Haha, thank you, but you made it easy. Yes, it appears the underscore in "3542281-improve-charts_chartjs-readme" might be what caused the issue.
andileco → created an issue.
@jofitz - the infrastructure team though the underscores in the branch name may have been the issue, so I created a MR to test. However, I will commit yours.
Going to reach out to the infrastructure team so that I can do this through the D.O. interface. Thanks!
I think this is just the commit, not the MR - can you make sure you've submitted the MR? That way the code quality steps run. If those pass, this looks good to me.
andileco → created an issue.
Needed a slight tweak if you turn the CDN back on.
Thank you! Going to issue a new release.
Thank you! I will check and issue a release tonight or tomorrow.
andileco → created an issue.
Spotted two minor PHPCS issue - fixing those.
Tested the various operators included in the numeric field. Also checked that the existing filters continued to work as expected.
andileco → created an issue.
andileco → created an issue.