feat: add issue and pr related interaction #524

andyhuang18 · 2022-11-16T14:51:11Z

Brief Information

This pull request is in the type of (more info about types):

Related issues (all available keywords):

resolve [Feature] Issue and pull request related interactions #519

Details

Checklist

I have read the CONTRIBUTING doc
I have signed the CLA

Others

tyn1998 · 2022-11-17T05:16:40Z

Hi @andyhuang18, I think I have found the reason why the chart does not respond to click event, finally!

Click events of line series chart can only be fired when we click on points rather lines:

However the points at the moment are set to none. If we set the symbol option to circle like above, we can click it.

andyhuang18 · 2022-11-17T05:44:16Z

Hi @tyn1998 ,Thanks for helping~ What a interesting difference between bar chart and line chart! Now that I am aware of this reason, I will continue working :)

tyn1998 · 2022-11-17T05:47:04Z

Enabling points will make the chart ugly in my view. Another way is to make the tooltip enterable and add code for interaction in the tooltip. However, it seems hard for users to enter the tooltip (they may try several times before succeeds entering it), like the video shows in below:

2022-11-17.13.38.03.mov

Hi @frank-zsy, what do you think? Which way do you prefer, or just close the issue?

andyhuang18 · 2022-11-17T06:02:54Z

Enabling points will make the chart ugly in my view. Another way is to make the tooltip enterable and add code for interaction in the tooltip. However, it seems hard for users to enter the tooltip (they may try several times before succeeds entering it), like the video shows in below:

2022-11-17.13.38.03.mov
Hi @frank-zsy, what do you think? Which way do you prefer, or just close the issue?

Looks like it is hard for mouse to catch up the tooltip.

frank-zsy · 2022-11-17T08:44:59Z

Click tooltip is not a good idea. So there is no event trigger when clicking on the line without enabling the points?

tyn1998 · 2022-11-17T10:10:51Z

Yes. I think that is because a line does not represent a time point.

andyhuang18 · 2022-11-17T10:25:00Z

I find that there is a triggerLineEvent

tyn1998 · 2022-11-17T10:27:38Z

That is so great!

https://echarts.apache.org/zh/option.html#series-line.triggerLineEvent

andyhuang18 · 2022-11-17T13:25:31Z

I'm wondering why the click function can't receive parameters when I click the line, however the tooltipFormatter can receive parameters.

In tooltipFormatter

So tooltip can display year-month information.

tyn1998 · 2022-11-17T13:36:58Z

Hi @andyhuang18, could you push a commit to this branch so I can have a look into the codebase.

andyhuang18 · 2022-11-17T13:41:16Z

These are parameters of onclick function. There isn't any data.

When I click the point, the function can receive the correct parameters. As the screenshot below

andyhuang18 · 2022-11-17T13:42:56Z

Hi @andyhuang18, could you push a commit to this branch so I can have a look into the codebase.

Sure~

andyhuang18 · 2022-11-17T13:57:33Z

I can interact with the point of issue chart right now.

tyn1998 · 2022-11-17T14:22:32Z

The click event on line seems provide no detailed data, as you mentioned. However, the effect of your code is good. Though symbol is set, but with symbolSize set to a small value, it will only be visible when you hovering on it, which is really wanted！

2022-11-17.22.18.41.mov

You can apply this method to another chart :)

andyhuang18 · 2022-11-17T14:37:28Z

The click event on line seems provide no detailed data, as you mentioned. However, the effect of your code is good. Though symbol is set, but with symbolSize set to a small value, it will only be visible when you hovering on it, which is really wanted！

2022-11-17.22.18.41.mov
You can apply this method to another chart :)

Copy that~ :)

andyhuang18 · 2022-11-17T17:11:36Z

Now we can interact with both point and line~ Though my solution maybe is not the perfect one...

tyn1998 · 2022-11-18T03:52:25Z

@andyhuang18, you are sooooo smart! Nice workaround!

The solution is very good while there is still room for functional improvments:

to allow user to click open line to visit Issues/PR list filtered by "created:xxxx-xx"
to allow user to click close/merge line to visit Issues/PR list filtered by "closed:xxxx-xx"/"merged:xxxx-xx"
comment/review line → "updated:xxxx-xx"

So the charts will be more flexible in interactions. As to the choice of filters, you may choose proper filters to replace the ones I mentioned above.

…8/hypertrons-crx into feat/add_issue_pr_interaction # Conflicts: # src/views/RepoDetailIssueView/IssueChart.tsx # src/views/RepoDetailIssueView/RepoDetailIssueView.tsx

frank-zsy · 2022-11-18T07:02:11Z

Great work, guys! Totally what I want.

tyn1998

Great job! @andyhuang18

Almost done, but with some suggestions here.

tyn1998 · 2022-11-18T08:34:53Z

src/views/RepoDetailIssueView/IssueChart.tsx

    }
  }, []);

  return <div ref={divEL} style={{ width, height }}></div>;
 };

+let issueclickparams: { data: number[]; marker: any; seriesName: any };


Since only issueclickparams.data[0] is read as ym, maybe we can use a variable called curMonth to store the property and pass the string variable to onClick(curMonth, params) directly.

In my view this is more clear.

tyn1998 · 2022-11-18T08:35:56Z

src/views/RepoDetailIssueView/IssueChart.tsx

 const tooltipFormatter = (params: any) => {
+  issueclickparams = params[0];


So this line will be curMonth = params[0].data[0];

Note: in JavaScript variables are conventionally named in lowerCamelCase.

src/views/RepoDetailIssueView/RepoDetailIssueView.tsx

tyn1998

LGTM, thank you @andyhuang18!

/approve

andyhuang18 added 3 commits November 16, 2022 20:24

test

8a80032

test for issue chart

341e9bd

test2 for issue chart

6e82b26

add point symbol->circle and triggerLineEvent->true

189e257

feat: add issue and pr related interaction(final version)

5a306be

Merge branch 'feat/add_issue_pr_interaction' of github.com:andyhuang1…

95e6ff5

…8/hypertrons-crx into feat/add_issue_pr_interaction # Conflicts: # src/views/RepoDetailIssueView/IssueChart.tsx # src/views/RepoDetailIssueView/RepoDetailIssueView.tsx

feat: more specific interactions with line charts

d5b71c3

tyn1998 requested changes Nov 18, 2022

View reviewed changes

andyhuang18 added 2 commits November 18, 2022 17:24

feat: more specific interactions with line charts(modified)

b5ebcda

feat: more specific interactions with line charts(modified again)

21ac5f0

tyn1998 approved these changes Nov 18, 2022

View reviewed changes

menbotics bot added the pull/approved If a pull is approved, it will be automatically merged label Nov 18, 2022

tyn1998 marked this pull request as ready for review November 18, 2022 10:49

tyn1998 changed the title ~~WIP: feat: add issue and pr related interaction~~ feat: add issue and pr related interaction Nov 18, 2022

menbotics bot merged commit 8052b5d into hypertrons:master Nov 18, 2022

tyn1998 mentioned this pull request Nov 18, 2022

[Feature] Issue and pull request related interactions #519

Closed

andyhuang18 deleted the feat/add_issue_pr_interaction branch March 14, 2023 02:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add issue and pr related interaction #524

feat: add issue and pr related interaction #524

andyhuang18 commented Nov 16, 2022 •

edited by tyn1998

Loading

tyn1998 commented Nov 17, 2022 •

edited

Loading

andyhuang18 commented Nov 17, 2022

tyn1998 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022

frank-zsy commented Nov 17, 2022

tyn1998 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022

tyn1998 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022

tyn1998 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022 •

edited

Loading

andyhuang18 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022

tyn1998 commented Nov 17, 2022 •

edited

Loading

andyhuang18 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022 •

edited

Loading

tyn1998 commented Nov 18, 2022 •

edited

Loading

frank-zsy commented Nov 18, 2022

tyn1998 left a comment

tyn1998 Nov 18, 2022

tyn1998 Nov 18, 2022

tyn1998 left a comment

		const tooltipFormatter = (params: any) => {
		issueclickparams = params[0];

feat: add issue and pr related interaction #524

feat: add issue and pr related interaction #524

Conversation

andyhuang18 commented Nov 16, 2022 • edited by tyn1998 Loading

Brief Information

Details

Checklist

Others

tyn1998 commented Nov 17, 2022 • edited Loading

andyhuang18 commented Nov 17, 2022

tyn1998 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022

frank-zsy commented Nov 17, 2022

tyn1998 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022

tyn1998 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022

tyn1998 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022 • edited Loading

andyhuang18 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022

tyn1998 commented Nov 17, 2022 • edited Loading

andyhuang18 commented Nov 17, 2022

andyhuang18 commented Nov 17, 2022 • edited Loading

tyn1998 commented Nov 18, 2022 • edited Loading

frank-zsy commented Nov 18, 2022

tyn1998 left a comment

Choose a reason for hiding this comment

tyn1998 Nov 18, 2022

Choose a reason for hiding this comment

tyn1998 Nov 18, 2022

Choose a reason for hiding this comment

tyn1998 left a comment

Choose a reason for hiding this comment

andyhuang18 commented Nov 16, 2022 •

edited by tyn1998

Loading

tyn1998 commented Nov 17, 2022 •

edited

Loading

andyhuang18 commented Nov 17, 2022 •

edited

Loading

tyn1998 commented Nov 17, 2022 •

edited

Loading

andyhuang18 commented Nov 17, 2022 •

edited

Loading

tyn1998 commented Nov 18, 2022 •

edited

Loading