GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Add support for dashed marker lines in scatter plots by chrimaho · Pull Request #7673 · plotly/plotly.js Conversation
|
| current state | target state |
|---|---|
![]() |
![]() |
Closes #7667
Sorry, something went wrong.
There was an error while loading. Please reload this page.
- Enable `marker.line.dash` attribute in scatter trace definitions with support for array values - Apply dash patterns to marker outlines in both open and closed marker styles - Merge dash data into calcdata for per-point dash styling - Include dash information in legend marker rendering to match trace appearance - Default dash handling falls back to trace-level `marker.line.dash` when per-point values are not specified
- Document new feature that adds support for dashed marker lines in scatter plots - Reference pull request plotly#7673 for tracking purposes
- Extends marker line styling capabilities by including the `dash` attribute alongside existing `width` attribute - Enables dashed marker line borders for `scatter3d`, `scattercarpet`, `scattergeo`, `scattergl`, `scatterternary`, and `splom` trace types - Maintains consistency with base scatter trace marker line attributes by reusing `scatterMarkerLineAttrs.dash` - Applies appropriate constraints where needed, such as `arrayOk: false` for 3D traces and `editType: 'calc'` for other trace types
- Prevent marker line borders from rendering on blank data points by setting line width to 0 when `d.isBlank` is true in the `singlePointStyle()` function - Add `dash` and `dashsrc` schema attributes for marker line styling across multiple chart types to support customisable dash patterns - Ensure blank points appear correctly without visible outlines while maintaining proper styling for non-blank markers
- Add `dash` property to marker line configurations across multiple plot types, allowing customisation of line dash patterns - Support both predefined dash styles (`solid`, `dot`, `dash`, `longdash`, `dashdot`, `longdashdot`) and custom dash length lists in pixels - Include `dashsrc` property for Chart Studio Cloud integration to enable data source references for dash styling - Set default value to `solid` for consistent backward compatibility - Enable array support for most implementations to allow per-marker dash customisation
- Simplify the dash line styling implementation by replacing the single `.style()` call with an object parameter with chained `.style()` method calls - Set `stroke-dasharray` to `null` when dash is falsy to properly clear the style rather than setting it to an empty or undefined value - Improve code readability through method chaining pattern
- Consolidate style application in `dashLine()` function to use object notation instead of chained method calls - Remove unused `lineDash` variable and associated `dashLine()` calls in marker styling logic - Eliminate redundant dash line styling for open markers and blank points that was not being applied correctly
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this PR together! You've done part of the work, but you'll need to update src/components/drawing/index.js to get the dashed lines to draw. On line 967, if you add the following, you should see the dashes:
const lineDash = d.mld || (markerLine || {}).dash; if (lineDash) drawing.dashLine(sel, lineDash, lineWidth);
Additionally, there are some traces that won't be compatible with this change due to not being supported in WebGL. Could you please remove the following traces from this PR?:
From my research, these traces seem like they would most benefit from this feature:
Could you add this feature to all of these traces?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Could you please remove this file from the PR? It only gets updated during releases.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in 8573067
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| }, | ||
| dash: extendFlat({}, dash, { | ||
| arrayOk: true, | ||
| editType: 'style' |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This property already exists on the dash object, so it isn't needed here.
| editType: 'style' |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in 6fa75bc
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
You removed the wrong line. arrayOk: true is still needed. editType: 'style' should be removed.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Corrected in 3dd16f7
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Please remove the changes for scatter3d. A dashed line isn't supported at the moment since it doesn't use SVG.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in ac01dc3
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It looks like the ending new line was deleted. Could you add it back please?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in f733505
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Please remove the changes for scattergl. A dashed line isn't supported at the moment since it doesn't use SVG.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in ac01dc3
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It looks like the ending new line was deleted. Could you add it back please?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in f733505
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Please remove the changes for splom. A dashed line isn't supported at the moment since it doesn't use SVG.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in ac01dc3
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It looks like the ending new line was deleted. Could you add it back please?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in f733505
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…& `splom` traces. Revert to the changes from the `master` branch
|
Thanks for the help @camdecoster! Really appreciate it.
I have updated this part ☝️ in commit f348454.
I have implemented this part ☝️ in commit ac01dc3
For traces For Thank you again, I really appreciate your support. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
There are a few more minor changes to make. And you're right, scattermap does use WebGL so that should be skipped. Could you please add a new test mock (or modify an existing one) to make sure the marker line dash is working?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| }, | ||
| dash: extendFlat({}, dash, { | ||
| arrayOk: true, | ||
| editType: 'style' |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
You removed the wrong line. arrayOk: true is still needed. editType: 'style' should be removed.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It looks like the ending new line was deleted. Could you add it back please?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It looks like the ending new line was deleted. Could you add it back please?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It looks like the ending new line was deleted. Could you add it back please?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
You'll also need to update the plot schema and commit that to fix the failing test. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@chrimaho are you still interested in finishing this PR? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
- In the `attributes.js` file for the traces `scatter3d`, `scattergl`, and `splom`
- Enable array support for the `dash` attribute. - Correct line from `editType: 'style'` to `arrayOk: true`
- Introduce a new mock data file for testing scatter marker line dash - Implement tests for marker line dash functionality - Validate support for array of dashes in markers - Ensure marker line dash is displayed in the legend - Confirm marker line dash updates via restyle - Test marker line dash on open markers
Done in 7eb1943 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Done in 90ab105 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Hi @camdecoster, thanks for the bump. Yes, I'm still very keen on this PR. Thank you kindly. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
This reverts commit d4d4972.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@chrimaho I went down a rabbit hole trying to get one of the new tests to pass, which led to a number of updates. Ultimately, I think this PR is ready to go. For now, the legend markers associated with shape traces won't show the line dash. This was deemed to be a "breaking" change and will be added in a future major revision. But the rest of it works as you intended. Thanks for the contribution!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@camdecoster, thank you so much! Really appreciate your support 👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
[FEATURE]: Allow dashed borders for scatter plot markers (marker.line.dash)
There was an error while loading. Please reload this page.