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

Split Modules #26

Closed
2 of 9 tasks
curran opened this issue Jan 13, 2020 · 11 comments
Closed
2 of 9 tasks

Split Modules #26

curran opened this issue Jan 13, 2020 · 11 comments

Comments

@curran
Copy link
Contributor

curran commented Jan 13, 2020

Part of #15, the goal here is to split up large JS files ( > 500 lines) into smaller modules.

  • viz/helpers/ui/legend.js Split Legend Module #31
  • viz/types/helpers/graph/includes/svg.js
  • viz/helpers/shapes/draw.js
  • viz/helpers/shapes/edges.js
  • viz/helpers/ui/timeline.js
  • tooltip/create.js
  • geom/largestrectangle.js
  • viz/types/helpers/graph/includes/plot.js
  • viz/types/rings.js
@curran
Copy link
Contributor Author

curran commented Jan 13, 2020

@pachamaltese Regarding splitting modules, I'm thinking to tackle the largest files only and leave the rest. There are 9 files that are over 500 lines of code. I think limiting the scope of this task to just these files is a good idea. Thoughts?

  1     751 ./src/viz/helpers/ui/legend.js
  2     748 ./src/viz/types/helpers/graph/includes/svg.js
  3     738 ./src/viz/helpers/shapes/draw.js
  4     688 ./src/viz/helpers/shapes/edges.js
  5     664 ./src/viz/helpers/ui/timeline.js
  6     601 ./src/tooltip/create.js
  7     558 ./src/geom/largestrectangle.js
  8     556 ./src/viz/types/helpers/graph/includes/plot.js
  9     501 ./src/viz/types/rings.js

FWIW the command to get this list is find . -type f -exec wc -l {} + | sort -rn | head

@curran curran mentioned this issue Jan 13, 2020
5 tasks
@pachadotdev
Copy link
Owner

@curran I agree. I'll post an idea in regards to find

@curran
Copy link
Contributor Author

curran commented Jan 22, 2020

@pachamaltese I'm noticing that there are no examples that use the legend component, so if I refactor that file there is no way to verify whether it breaks something or not. How would you like me to proceed?

@pachadotdev
Copy link
Owner

pachadotdev commented Jan 22, 2020 via email

@pachadotdev
Copy link
Owner

pachadotdev commented Jan 22, 2020 via email

@curran
Copy link
Contributor Author

curran commented Jan 22, 2020

I can remove the legend module completely without breaking any of the examples that are there with HTML.

image

I wonder, do you have any R-based examples that use the legend that could be easily ported to JavaScript?

I'm happy to just remove the legend component if you don't want the library to be able to make legends.

@pachadotdev
Copy link
Owner

pachadotdev commented Jan 22, 2020 via email

@pachadotdev
Copy link
Owner

pachadotdev commented Jan 22, 2020 via email

@pachadotdev
Copy link
Owner

@curran I added a very stupid example here https://github.com/pachamaltese/d3po/blob/master/dev/treemap_with_legend.html

I shall port legends after remembering that 3 yrs ago I used those a lot

image

@curran
Copy link
Contributor Author

curran commented Jan 23, 2020

Excellent! Thank you for adding that.

@curran
Copy link
Contributor Author

curran commented Jan 28, 2020

Legend refactoring ready for review #31

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

No branches or pull requests

2 participants