-
Notifications
You must be signed in to change notification settings - Fork 102
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: Design and implement several variable charts #716
Conversation
Great! Wish you have a good time in your trip. 😄 |
Hi, @wj23027 . I have changed this base branch to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @wj23027, I found that the GridLayout is not imported by any file so I guess the screenshot you provided in the PR description is just for preview. Since @wxharry provided a way to test your charts, you can import GridLayout there.
Besides, please comment the code in English and hurry up :D
@wxharry , thank you for your suggestions and the method you provided for testing the new charts! I have now developed some of the charts(as shown below). Next, I will continue to work on the boxplot and Sankey chart with Andy~ |
…light Specific Charts by Clicking the Menu Bar'
In addition, a simple grid layout component has been implemented to display the charts separately in both the modal and option pages. Furthermore, clicking on menu items can highlight data for corresponding repositories in certain charts. Next Tasks :
|
@wj23027 The Sankey chart is really good. There are only two repos in the demo collection, could you add more repos into it? Please bare in mind that these charts should be scalable for any given collection of different size. |
Hello @wj23027, could you add more repos into your mock data? Since in my view, these two charts are not a good choice for those collections with 3+ repos. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @wj23027, thanks for your time and effort contributed to this OSPP project. Here are some suggestions for you and I will give more of them in the following days.
In this update, I added numerical display panels.I tried increasing the number of repositories in the collection and noticed that after the increase, the code additions and deletions chart as well as the multi-dimensional bar chart didn't look very aesthetically pleasing. So, I made the following adjustments:
|
Additionally, I've noticed that when the collection contains too many developers, there are some issues with the Sankey chart's display. Therefore, I've increased the size of the Sankey chart, but it still works best for collections containing no more than 150 developers (ideally, fewer than 100). |
I have an issue to address: in the stacked bar chart, the data from different series isn't stacking as intended, but I haven't found the reason yet. |
Great! I am glad you tested it. Then we should add a warning or notice like when users are trying to add a sankey chart. I prefer adding it anyway since this is a very cool chart. What do you think @tyn1998 ?
It is because the two data arraies have different lengths. I simply sliced the data arraies to the latest 37 elements and it works. I think we need additional functions to fill up what's missing to 0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @wj23027 !
Your work LGTM, thank you for your contributions!
Brief Information
This pull request is in the type of (more info about types):
Related issues (all available keywords):
Details
I have implemented a basic grid layout for the charts, allowing different-sized charts to be added to the layout.
In the next steps, I will need to add logic to allow users to add charts of specific sizes and types to specific positions, and optimize the styling of the layout.
Next Steps:
Checklist
Others