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

DataCursor demo #18650

Merged
merged 8 commits into from Apr 3, 2023
Merged

DataCursor demo #18650

merged 8 commits into from Apr 3, 2023

Conversation

bre1470
Copy link
Contributor

@bre1470 bre1470 commented Mar 10, 2023 •
edited

Based on PR #18642

@bre1470 bre1470 self-assigned this Mar 10, 2023
@highsoft-bot
Copy link
Collaborator

File size comparison

No differences found

@highsoft-bot
Copy link
Collaborator

highsoft-bot commented Mar 10, 2023 •
edited

Visual test results - No difference found

Samples changed

Change type Sample
Modified samples/highcharts/data-tools/datacursor-html/demo.js
Modified samples/highcharts/data-tools/datacursor-html/demo.js
Modified samples/highcharts/data-tools/datacursor-html/demo.js
Added samples/highcharts/data-tools/datacursor-html/demo.js

goransle
goransle approved these changes Mar 13, 2023
View reviewed changes
Copy link
Member

@goransle goransle left a comment

Choose a reason for hiding this comment

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

Looks pretty clean to me

karolkolodziej
karolkolodziej approved these changes Mar 13, 2023
View reviewed changes
samples/highcharts/data-tools/datacursor-html/demo.js

// Synchronize MouseOver

const cursor = new Highcharts.DataCursor();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it going to be on Highcharts names as it is now or will it be part of the dashboards?

@bre1470 bre1470 requested a review from pawelfus March 16, 2023 09:58
pawelfus
pawelfus approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@pawelfus pawelfus left a comment

Choose a reason for hiding this comment

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

Thanks!

Shall we bring this demo to the next workshop session, too? 🤔 It's the first time I see Cursor in usage

@bre1470
Copy link
Contributor Author

bre1470 commented Mar 16, 2023

@pawelfus It is the second demo. Here you can find the first demo: https://github.com/highcharts/highcharts/tree/dashboards/main/samples/dashboards/demos/datacursor-sync

@bre1470 bre1470 marked this pull request as ready for review March 17, 2023 14:22
pawellysy
pawellysy approved these changes Mar 17, 2023
View reviewed changes
Copy link
Member

@pawellysy pawellysy left a comment

Choose a reason for hiding this comment

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

Great work!

TorsteinHonsi
TorsteinHonsi reviewed Mar 20, 2023
View reviewed changes
Copy link
Collaborator

@TorsteinHonsi TorsteinHonsi left a comment

Choose a reason for hiding this comment

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

Looks good! Maybe this is covered by the documentation, but for these advanced demos I am missing a short explanation on what the purpose of the demo is. What are we looking at, and why does it matter? This would be useful for people finding this demo randomly from the internet, who are not directed here from the docs.

PS: Is there a specific reason for not using the data grid component here?

@bre1470
Copy link
Contributor Author

bre1470 commented Mar 20, 2023 •
edited

@TorsteinHonsi
I was heading for the simplest demo possible, and for now DataGrid is also only part of Dashboards.

@bre1470 bre1470 marked this pull request as draft March 21, 2023 09:45
@bre1470 bre1470 marked this pull request as ready for review April 3, 2023 10:55
@bre1470 bre1470 merged commit e68e430 into master Apr 3, 2023
9 checks passed
@highsoft-bot highsoft-bot added this to the Next milestone Apr 3, 2023
@bre1470 bre1470 deleted the demo/datacursor-html branch April 3, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TorsteinHonsi TorsteinHonsi TorsteinHonsi approved these changes

@pawelfus pawelfus pawelfus approved these changes

@sebastianbochan sebastianbochan sebastianbochan approved these changes

@goransle goransle goransle approved these changes

@pawellysy pawellysy pawellysy approved these changes

@karolkolodziej karolkolodziej karolkolodziej approved these changes

Assignees

@bre1470 bre1470

Labels
None yet
Projects
None yet
Milestone
v11.0.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@bre1470 @highsoft-bot @TorsteinHonsi @pawelfus @sebastianbochan @goransle @pawellysy @karolkolodziej