Skip to content

Feat: add free draw option in plots and setup update pipeline for plots - #1563

Merged
Saksham-Sirohi merged 20 commits into
fossasia:devfrom
rajnisht7:feat-add-free-draw-update-pipeline
Jul 8, 2026
Merged

Feat: add free draw option in plots and setup update pipeline for plots#1563
Saksham-Sirohi merged 20 commits into
fossasia:devfrom
rajnisht7:feat-add-free-draw-update-pipeline

Conversation

@rajnisht7

@rajnisht7 rajnisht7 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds an option in plotly modbar with which user can directly use a marker to draw information on pltots, and can erase it, apart from this, this PR also introduces a plot layout update pipeline.
all the drawings made on the PR, persist after closing the server too

Motivation and Context

adding a free draw option so that user can draw anything on chart

How Has This Been Tested?

started the server
made some drawings
closed the server
loaded the env
verified that drawing remained as it it

Screencast_20260706_025132.online-video-cutter.com.mp4

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code refactor or cleanup (changes to existing code for improved readability or performance)

Checklist:

  • I adapted the version number under py/visdom/VERSION according to Semantic Versioning
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Copilot AI review requested due to automatic review settings July 5, 2026 21:14

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry @rajnisht7, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rajnisht7

Copy link
Copy Markdown
Contributor Author

Comparison tests are supposed to fail as this PR introduces visual changes

@Manik-Khajuria-5 Manik-Khajuria-5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rajnisht7 Since u are working on this PR please make this pr as draft as it will be good feat and will be having lot of implementation changes

@rajnisht7

Copy link
Copy Markdown
Contributor Author

@rajnisht7 Since u are working on this PR please make this pr as draft as it will be good feat and will be having lot of implementation changes

@Manik-Khajuria-5 as discussed, this PR is good to go, kindly review it

@Manik-Khajuria-5 Manik-Khajuria-5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rajnisht7 LGTM! I have read backend changes and they are fine Please address failing test rest everything is good to go

@rajnisht7

Copy link
Copy Markdown
Contributor Author

@rajnisht7 LGTM! I have read backend changes and they are fine Please address failing test rest everything is good to go

@Manik-Khajuria-5 Thanks for the review, the test is supposed to fail as this PR introduces visual changes

@tonypzy tonypzy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please resolve sourcery issues
js/api/ApiProvider.js:210 - User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities
js/api/ApiProvider.js:222 - User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities

@rajnisht7

rajnisht7 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

please resolve sourcery issues js/api/ApiProvider.js:210 - User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities js/api/ApiProvider.js:222 - User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities

@tonypzy these are completely un-related to this PR neither these changes are introduced by this PR, we can keep this PR's scope limited to this feature only

this PR introduces changes from line 323 to 332 and 391
and have no relation with line 210 and 222

@Saksham-Sirohi
Saksham-Sirohi requested a review from tonypzy July 8, 2026 14:10
@Saksham-Sirohi
Saksham-Sirohi merged commit 510fc9c into fossasia:dev Jul 8, 2026
10 of 11 checks passed
@Saksham-Sirohi

Copy link
Copy Markdown
Collaborator

Please fix issues flagged by sorcery in a seperate pr

@rajnisht7

Copy link
Copy Markdown
Contributor Author

Please fix issues flagged by sorcery in a seperate pr

Sure, will file a PR for the same. Thank You

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants