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

feat(network): add network graph #435

Closed
wants to merge 88 commits into from

Conversation

j1mie
Copy link
Contributor

@j1mie j1mie commented Dec 12, 2019

This is an initial PR for the network graph component.

There is still outstanding work to complete for expansion, mini map etc. however in order to get the ball rolling and keep PR sizes reasonable, I can submit these as a series of PRs, if that's all good with you guys 🙌 (I've left a list out upcoming items at the bottom of the PR description)

Adds

  • Network graph demo data, and demo to homepage
  • @carbon/icons and @carbon/icon-helpers dependencies to support the rendering of icons in the network graph
  • Network chart class at packages/core/src/charts/network.ts
  • Index file for network component at packages/core/src/components/graphs/network/index.ts
  • network-card sub component at packages/core/src/components/graphs/network/network-card.ts , a function that renders out the card component
  • network-line sub component at packages/core/src/components/graphs/network/network-line.ts, a function that renders out lines components and their markers
  • network component at packages/core/src/components/graphs/network/network.ts that calls both the network line and network card functions, and also calls d3 zoom function
  • A path building utility using d3-path for correct spacing of lines between cards, at packages/core/src/components/graphs/network/utils/build-path-string.ts
  • An icon path utility function for rendering icons at packages/core/src/components/graphs/network/utils/get-icon-string.ts
  • Basic interfaces for Network and Zoomable charts
  • SCSS styles for network, network-card, network-line at packages/core/src/styles/graphs/network/
  • Zoomable chart class

Changed

  • Formatting and check for type.options in packages/core/demo/index.ts
  • Export network graph component at packages/core/src/components/index.ts
  • Network chart and Zoomable chart configuration options at packages/core/src/configuration.ts
  • Export the Network chart at packages/core/src/index.ts

Questions / Comments

  • I'm unsure of the best approach to extend the Zoomable chart class and would appreciate guidance, feedback here?
  • How can I remove the "Update data" button on the demo page, or what way would make sense to use this in the context of the network graph?
  • Can I mark the network example on the demo page as "Experimental" or "WIP" and if so, what the best approach for doing this?
  • The SCSS follows conventions from Carbon, and works fine with theming, is this acceptable in the context of this repo where theming may function differently?
  • Sub components are defined as functions / functional d3 components. Is this acceptable in the context of this repo or would you prefer to go with a class-based approach?
  • How do I test this component / add this component to React, Angular and Vue storybooks?

Todos / outstanding work

  • Expandable cards styling, logic
  • Dynamic x,y values based on rows and columns
  • Mini map component to complement zooming functionality
  • Document component, API
  • Text truncation
  • Pass custom components for cards / lines via render a prop pattern / function

Demo screenshot or recording

Network chart (white theme)
Screenshot 2019-12-12 at 10 51 16

Network chart / focussed card (white theme)
Screenshot 2019-12-12 at 10 51 21

Network chart (g10 theme)
Screenshot 2019-12-12 at 10 51 35

Network chart (g90 theme)
Screenshot 2019-12-12 at 10 51 45

Network chart (g100 theme)
Screenshot 2019-12-12 at 10 51 56

Pan, drag, zoom behaviour
Screen Recording 2019-12-12 at 10 54 32

Review checklist (for reviewers only)

  • Demos all features
  • Documented/annotated
  • Matches UI/UX specs
  • Meets the code style guide
  • Accessible
  • Mobile first (responsive)
  • RTL support (bidirectional text)
  • Performant (limited bloat)

@theiliad
Copy link
Member

theiliad commented Feb 13, 2020

@j1mie I think the core netlify deploy preview is still broken. Same error on Travis

@j1mie
Copy link
Contributor Author

j1mie commented Feb 17, 2020

@theiliad gotcha. Not seeing the Travis error after merging with master.

@theiliad
Copy link
Member

@theiliad gotcha. Not seeing the Travis error after merging with master.

Problem is I don't see your demo on the deploy preview now. Perhaps it's gone with the core demo site refactor?

@j1mie
Copy link
Contributor Author

j1mie commented Feb 17, 2020

@theiliad Refactored based on recent updates

@j1mie
Copy link
Contributor Author

j1mie commented Mar 12, 2020

@theiliad any updates on this

@theiliad
Copy link
Member

Hi Jimmy, still trying to get design to take a look at this component 😞
I'll ask the team again to take look

@designertyler
Copy link

Hi @j1mie there are couple things that stand out from the specs I saw from @cameroncalder

The network card heading is a little high and starts to look top aligned with the icon. This should appear center aligned with the icon.

image

The network link line uses a different style arrowhead and bend. Was this a later design change?

image

The network link line's color is using the older version of gray-50 that was #8c8c8c and is now #8d8d8d. The specs were using the older gray-30 which is a bit lighter. Double checking that wasn't another design decision after the specs were made.

I like the shorter 24px that are between the nods in the preview link, but the specs have a taller 32px between. Maybe this was another design change that was done in code and the specs not updated.
image

I would recommend going the full 80px between the nodes. Right now it looks like it's 76px at the default zoom level.
image

There's a slight overlap of the arrowhead and the node.
image

@stanislavgeorgiev
Copy link
Contributor

Is there an ETA on when would this work be completed? We could use this network chart in our project 👍

@keithcha
Copy link

Is there an ETA on when would this work be completed? We could use this network chart in our project 👍

Just bumping this for an update. It's been 9 months since this PR was opened.

@j1mie
Copy link
Contributor Author

j1mie commented Sep 25, 2020

Hi there, unfortunately these changes did not make it upstream, I'm going to close this out for the time being and will post here again if the situation changes.

@j1mie j1mie closed this Sep 25, 2020
@theiliad
Copy link
Member

Hi, based on the survey we ran 3 months ago, here's the current list of items based on priority

  1. KPI
  2. Histogram
  3. Decision tree
  4. Heatmap
  5. Bullet chart
  6. Treemap
  7. Cross tab
  8. Network diagram
  9. Box and whisker
  10. Alluvial
  11. Waterfall

Network diagram is currently number 8 on the list

@keithcha
Copy link

Hi, based on the survey we ran 3 months ago, here's the current list of items based on priority

  1. KPI
  2. Histogram
  3. Decision tree
  4. Heatmap
  5. Bullet chart
  6. Treemap
  7. Cross tab
  8. Network diagram
  9. Box and whisker
  10. Alluvial
  11. Waterfall

Network diagram is currently number 8 on the list

Thank you for the update!

@stanislavgeorgiev
Copy link
Contributor

Is there an experimental branch where people can contribute half baked solutions and get help from other people?

@theiliad theiliad reopened this Oct 1, 2020
@ninja511 ninja511 added this to QA/Review in Q2 - 2021 May 18, 2021
@Gunasoundari
Copy link

Gunasoundari commented Jun 10, 2021

Hi @j1mie, Any update on this PR?

@theiliad
Copy link
Member

Closing in favor of #1022

@theiliad theiliad closed this Jun 10, 2021
@ninja511 ninja511 linked an issue Jun 14, 2021 that may be closed by this pull request
@ninja511 ninja511 moved this from QA/Review to Closed in Q2 - 2021 Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Network diagram - Explorations
7 participants