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.

Sign up for GitHub

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

Jump to bottom

feat(visual): toggle toolchain visibility in performance visualization #569

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat(visual): toggle toolchain visibility in performance visualization #569

wants to merge 4 commits into from

Conversation

SiyaoIsHiding
Copy link

Regarding this issue #402

First, the json.gz file you give me will fail the make build process because some records do not have the property versions or family. Therefore, I added the following codes:

results['versions'] if 'versions' in results else {}, results['family'] if 'family' in results else {}, results['device']

Second, I tried to achieve the functionality to toggle the visibility by clicking the LINES instead of the legend, but I failed. I made a workaround to toggle by clicking the legend.

Something like this in the chart configuration won't work

interaction: {
        mode: 'index',
        intersect: false,
        axis: 'x',
        onHover: function (event, chartElement) {
          console.log("hover"); // no log
        },
        onClick: function (event, chartElement) {
          console.log("click");  // no log either
        }
      }

Something like this won't work either

function clickHandler(click){
    const points = myChart.getElementsAtEventForMode(click, 'nearest', { intersect: true}, true);
    console.log(points); // always an empty array
}
myChart.canvas.onclick = clickHandler;

Hope you find the workaround acceptable.

@tmichalak
Copy link
Collaborator

@SiyaoIsHiding thanks, I will take a look. A quick comment would be to fix the formatting errors reported by the CI .

@SiyaoIsHiding
Copy link
Author

@SiyaoIsHiding thanks, I will take a look. A quick comment would be to fix the formatting errors reported by the CI .

Thx for your advice. It should be able to pass the yapf format check now.

tmichalak
tmichalak requested changes Mar 27, 2023
View reviewed changes
Copy link
Collaborator

@tmichalak tmichalak left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SiyaoIsHiding thanks again for this PR, however this solution is not what we actually want to achieve.
The legend approach that you implemented is something that we had already some time ago. The objective of #402 is to add an option to select/deselect a line for a given toolchain from a context menu on that particular line (when you click on a specific data point). Besides, note that your current solution works for all graphs, not just the one that the legend belongs to.

@SiyaoIsHiding
Copy link
Author

@tmichalak
He says "disable ALL the lines corresponding to the particular toolchain." I assume he means disable the corresponding line in all the graphs.
If you want to only select/deselect a single line, that would be easy to achieve.
May I confirm what you want?

Besides, I would not be able to achieve the functionality that triggered by clicking a data point instead of a legend. I tried and I failed, as explained above.

@palaniappan-r palaniappan-r mentioned this pull request Feb 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmichalak tmichalak tmichalak requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SiyaoIsHiding @tmichalak