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

Issue5 upgrade dependencies d3plus #10

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

PaulBoon
Copy link
Collaborator

@PaulBoon PaulBoon commented Jul 6, 2023

Upgrades from d3plus v1 to v2.
Related PR: #9
Related Issue: #9

@PaulBoon
Copy link
Collaborator Author

PaulBoon commented Jul 6, 2023

Problem with the StackedArea:

Screenshot from 2023-07-06 16-16-29

@PaulBoon
Copy link
Collaborator Author

PaulBoon commented Jul 19, 2023

Just created an issue at d3plus for the stacked area plot problem; d3plus/d3plus#726

@PaulBoon
Copy link
Collaborator Author

Did manage to 'fix' most of the migration/porting problems:

  • Graphs have no footer, so most likely place configured text in html element below it.
  • Texts and formatting not migrated; tooltips can be improved
  • Links to datasets or files when applicable
  • colors from config not migrated, default seems acceptable
  • Missing tickmarks on x-axis for timeseries; seems acceptable when tooltips work
  • Lots of commented-out code that must be removed when done
  • Local code in directory d3plus.v1.9.8 should be removed, if we use CDN

Just not that "stacked (chart) not working properly".

Besides that I think it is acceptable as a replacement of the version with d3plusv1.9.8.
The 'plot' are behaving differently in showing labels, tick-marks etc., unfortunately I am not able to get that old behavior back. There are lots of issues with the dynamic resizing of the graphs when resizing the window, scrolling, collapse/expand sections... seems that d3plus would need some fixing for that.

The documentation and examples for are aimed at React programmers, so we might want to go that way in the future.
Also we might want to consider using D3js directly, instead of d3plus.

@PaulBoon
Copy link
Collaborator Author

PaulBoon commented Jul 20, 2023

Got an answer from d3plus (Dave Landry), with the fix:

.time("date")
.timeline(false)

Also going to apply this to the BarCharts with a time series.
We might consider adding a configuration option to show that timeline selection (timeline(true)), but not in this upgrading PR.
Screenshot from 2023-07-20 10-29-40

@PaulBoon PaulBoon marked this pull request as ready for review July 20, 2023 08:27
@PaulBoon
Copy link
Collaborator Author

PaulBoon commented Jul 20, 2023

Discovered a problem in the Chrome browser; The 'File by Type' graphs don't sort on count which results in a 'messy' graph:
Screenshot from 2023-07-20 16-07-12
It does work in FireFox:

Screenshot from 2023-07-20 16-13-11

@PaulBoon PaulBoon marked this pull request as draft July 20, 2023 14:26
@PaulBoon
Copy link
Collaborator Author

PaulBoon commented Jul 20, 2023

The StackedArea Was slow and not very useful with the d3plusv1, but with d3plusv2 it is making the whole metrics page unusable; not much is rendered and the browser suggest to stop the application...

The graph looks like this in the old (non-upgraded IQSS) version:
Screenshot from 2023-07-20 16-20-11

The workaround on our server is to disable the graph via the config; "multitimeseries.uniquedownloads": false,.

@PaulBoon
Copy link
Collaborator Author

PaulBoon commented Jul 25, 2023

Sorting problem might be fixable, tested another sorting function and on FF it remains working, just have to test Chrome.
The problem is that we now use a comparison that results in a boolean, while the sort wants a number.

We had:

.xSort(function(a,b) { return a["count"] < b["count"];})

If we replace it with this

.xSort(function(a,b) { return b["count"] - a["count"];})

It might work on Chrome.

@PaulBoon
Copy link
Collaborator Author

The sorting problem has been fixed, so we now only have problems with the StackedArea.

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.

1 participant