Fix: Flexline: label attributes are not working#1702
Conversation
f5a2c97 to
2c5ffac
Compare
martinRenou
left a comment
There was a problem hiding this comment.
Thanks for contributing!
There are many this as any introduced in this PR, can you please undo all those changes? Also none of the introduced package-lock.json and yarn.lock files should be kept here, and the test_flexline_label.ipynb should be removed and you may want to add some of its content in the examples folder at the right place (where we document flexline)
6de0854 to
bdada0b
Compare
|
Thanks for the review. I removed the introduced as any changes, dropped the lockfiles and test notebook from the PR, and kept the diff focused on the FlexLine fix. I also verified the remaining changes pass the relevant tests. If you'd prefer, I can follow up with a separate notebook example for FlexLine for label visibility. |
|
Hi @martinRenou I hope all is well. I just wanted to check in to let you know that I have taken a look at the feedback you provided for this PR. Feel free to let me know if there are additional changes needed to be made. |
|
Sorry I missed the first ping, thanks having a look |
|
Can you please run |
a41ab0f to
9695dd5
Compare
|
Hello @martinRenou, When running the global linting commands, I noticed a large number of pre-existing errors and warnings unrelated to my changes. To ensure my contribution meets the project standards without introducing 'noise' into the PR, I have:
|
|
Hello @martinRenou, I have removed the cell and ensured the notebooks are clean. I ran all the tests using the command pytest && flake8 bqplot/. This runs both the test suite and the linter in one go. All the tests passed, and the only linter messages were line length warnings, no errors or failures. I wasn't quite sure whether or not the warnings needed to be tackled or not, so I left that unless you think its need to be taken care of. |
- Add `labels_visibility` trait to FlexLine Python model and render inline curve labels at the end of each line in the JS view - Fix FlexLine.draw_legend to use the correct D3 enter selection and get_mark_color instead of get_colors - Fix TypeScript type errors in Lines.ts and FlexLine.ts with explicit casts Cleanup duplicate listeners for FlexLine Address review feedback: remove as-any churn and drop lock/notebook files docs: move Flexline notebook example into docs
d046c64 to
6f0a5fe
Compare
jellyfishing2346
left a comment
There was a problem hiding this comment.
Hello @martinRenou, I hope all is well. I am just checking to let you know that I have addressed the feedback you mentioned for the FlexLine notebook.
|
Hello @martinRenou, I hope all is well. I am just checking to let you know that I have addressed the feedback you mentioned for the FlexLine notebook. |
References
Fixes the FlexLine label rendering issue where the labels attribute was not displayed correctly.
Closes #1272
Currently there are no existing pull requests for this role
Code changes
Removed duplicate event listeners in FlexLine so the mark now updates its label state consistently. This prevents the label handling path from being overwritten or ignored when the mark is rendered.
User-facing changes
FlexLine marks now display the provided labels value correctly. This improves legend/label visibility when using display_legend=True.
Backwards-incompatible changes
None