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.

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

Render shape label while drawing #6608

Merged
merged 5 commits into from
May 19, 2023
Merged

Render shape label while drawing #6608

merged 5 commits into from
May 19, 2023

Conversation

emilykl
Copy link
Contributor

@emilykl emilykl commented May 18, 2023

Closes #6540

Render shape label text during initial shape draw.

Codepen demo here: https://codepen.io/emily_plotly/pen/eYLrKNV

This PR leverages the existing newshape code to render the shape label while the shape is being drawn.

@alexcjohnson @archmoj I'm very interested to get your feedback on whether this approach makes sense and whether there might be any unintended side effects.

Note: I moved the drawLabels() function from shapes/draw.js into its own file; that makes up the bulk of the changes in terms of # of lines. No changes to that function itself.

Outstanding changes:

  • Fix broken Jasmine tests
  • Make label partially transparent like the shape itself

Sorry, something went wrong.

@emilykl emilykl requested review from archmoj and alexcjohnson May 18, 2023 21:46
@@ -0,0 +1,268 @@
'use strict';
Copy link
Contributor Author

@emilykl emilykl May 18, 2023

Choose a reason for hiding this comment

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

This is just the drawLabels() function which used to be in shapes/draw.js being moved into its own file (in order to call it elsewhere without circular imports). No code changes.

@@ -70,8 +62,65 @@ module.exports = function newShapes(outlines, dragOptions) {
}
}

var isOpenMode = openMode(dragmode);
var newShape = createShapeObj(outlines, dragOptions);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only actual change here is moving some of the logic into a separate createShapeObj() function

@archmoj archmoj added bug something broken feature something new status: has TODOs labels May 18, 2023
@archmoj
Copy link
Contributor

archmoj commented May 19, 2023

Nicely done.
/ :dancer: /
\ 💃 \

@emilykl
Copy link
Contributor Author

emilykl commented May 19, 2023

Tagging @JulianWgs as well!

@archmoj archmoj merged commit 6607870 into master May 19, 2023
@archmoj archmoj deleted the shape-label-while-drawing branch May 19, 2023 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Render newshape text and texttemplate during initial shape draw
2 participants