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

fix(getFillColor):data parameter missing from the arguments #1736

Merged
merged 4 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/core/src/components/graphs/alluvial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,11 @@ export class Alluvial extends Component {
if (isGradientAllowed) {
return `url(#${this.gradient_id}-link-${d.index})`
}

return this.model.getFillColor(d.source.name)
return this.model.getFillColor(d.source.name, undefined, {
...d,
source: d.source.name,
target: d.target.name
})
})
.attr('stroke-width', (d: any) => Math.max(1, d.width))
.style('stroke-opacity', alluvialConfigs.opacity.default)
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/components/graphs/area-stacked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export class StackedArea extends Component {
const self = this
const options = this.getOptions()
const { groupMapsTo } = options.data

const percentage = Object.keys(options.axes).some(axis => options.axes[axis].percentage)

const stackedData = this.model.getStackedData({
Expand Down Expand Up @@ -73,7 +72,9 @@ export class StackedArea extends Component {
originalClassName: 'area'
})
)
.style('fill', (d: any) => self.model.getFillColor(getProperty(d, 0, groupMapsTo)))
.style('fill', (d: any) =>
self.model.getFillColor(getProperty(d, 0, groupMapsTo), undefined, d)
Copy link
Member

Choose a reason for hiding this comment

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

we shouldn't send undefined over.

Here's an example in scatter.ts

return this.model.getFillColor(d[groupMapsTo], d[domainIdentifier], d)

Let's try to send all of that info over pls, and in case we can't locate the data, use null instead of undefined pls

Copy link
Member

Choose a reason for hiding this comment

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

This applies to all the files in the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://charts.carbondesignsystem.com/documentation/classes/Chart.html
the charts in this list don't have access to cartesianScale hence I couldn't use getDomainIdentifier with those and some of the charts receive binned and stacked data. I'll be sending null as mentioned.

)
.attr('role', Roles.GRAPHICS_SYMBOL)
.attr('aria-roledescription', 'area')
.attr('aria-label', (d: any) => getProperty(d, 0, groupMapsTo))
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/components/graphs/area.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export class Area extends Component {
originalClassName: 'area'
})
)
.style('fill', (group: any) => self.model.getFillColor(group.name))
.style('fill', (group: any) => self.model.getFillColor(group.name, undefined, group.data))
.transition()
.call((t: any) =>
this.services.transitions.setupTransition({
Expand All @@ -216,7 +216,9 @@ export class Area extends Component {
if (boundsEnabled) {
enteringAreas
.attr('fill-opacity', areaConfigs.opacity.selected)
.style('stroke', (group: any) => self.model.getStrokeColor(group.name))
.style('stroke', (group: any) =>
self.model.getStrokeColor(group.name, undefined, group.data)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.model.getStrokeColor(group.name, undefined, group.data)
self.model.getStrokeColor(group.name, null, group.data)

)
.style('stroke-dasharray', '2, 2')
.attr('stroke-width', 0.7 + 'px')
}
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/components/graphs/bar-grouped.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,10 @@ export class GroupedBar extends Bar {
originalClassName: 'bar'
})
)
.style('fill', (d: any) => this.model.getFillColor(d[groupMapsTo]))
.style('fill', (d: any) => {
const domainIdentifier = this.services.cartesianScales.getDomainIdentifier(d)
return this.model.getFillColor(d[groupMapsTo], d[domainIdentifier], d)
})
.attr('d', (d: any) => {
/*
* Orientation support for horizontal/vertical bar charts
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/components/graphs/bar-simple.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ export class SimpleBar extends Bar {
originalClassName: 'bar'
})
)
.style('fill', (d: any) => this.model.getFillColor(d[groupMapsTo]))
.style('fill', (d: any) => {
const domainIdentifier = this.services.cartesianScales.getDomainIdentifier(d)
return this.model.getFillColor(d[groupMapsTo], d[domainIdentifier], d)
})
.attr('d', (d: any) => {
/*
* Orientation support for horizontal/vertical bar charts
Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/components/graphs/bar-stacked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ export class StackedBar extends Bar {
originalClassName: 'bar'
})
)
.style('fill', (d: any) => this.model.getFillColor(d[groupMapsTo]))
.style('fill', (d: any) =>
this.model.getFillColor(d[groupMapsTo], d.data.sharedStackKey, d.data)
)
.attr('d', (d: any) => {
const key = d.data.sharedStackKey as string

Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/components/graphs/bullet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,10 @@ export class Bullet extends Component {
originalClassName: 'bar'
})
)
.style('fill', (d: any) => this.model.getFillColor(d[groupMapsTo]))
.style('fill', (d: any) => {
const domainIdentifier = this.services.cartesianScales.getDomainIdentifier(d)
return this.model.getFillColor(d[groupMapsTo], d[domainIdentifier], d)
})
.attr('d', (d: any) => {
/*
* Orientation support for horizontal/vertical bar charts
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/components/graphs/circle-pack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ export class CirclePack extends Component {
: `node node-leaf ${originalClass}`
})
})
.style('fill', (d: any) => this.model.getFillColor(d.data.dataGroupName))
.style('stroke', (d: any) => this.model.getFillColor(d.data.dataGroupName))
.style('fill', (d: any) => this.model.getFillColor(d.data.dataGroupName, undefined, d.data))
.style('stroke', (d: any) => this.model.getFillColor(d.data.dataGroupName, undefined, d.data))
.attr('cx', (d: any) => d.x)
.attr('cy', (d: any) => d.y)
.transition('circlepack-leaf-update-enter')
Expand Down Expand Up @@ -123,7 +123,7 @@ export class CirclePack extends Component {
this.parent
.selectAll('circle.node')
.filter((d: any) => data.some((datum: any) => datum === d.data) && d.depth > 1)
.style('stroke', (d: any) => this.model.getFillColor(d.data.dataGroupName))
.style('stroke', (d: any) => this.model.getFillColor(d.data.dataGroupName, undefined, d.data))
}

// highlight the children circles with a stroke
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/components/graphs/heatmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,12 @@ export class Heatmap extends Component {
.attr('width', this.xBandwidth)
.attr('height', this.yBandwidth)
.style('fill', (d: any) => {
const domainIdentifier = this.services.cartesianScales.getDomainIdentifier(d)
// Check if a valid value exists
if (d.index === -1 || d.value === null) {
return `url(#${patternID})`
}
return this.model.getFillColor(Number(d.value))
return this.model.getFillColor(Number(d.value), d[domainIdentifier], d)
})
.attr('aria-label', (d: any) => d.value)

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/graphs/histogram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export class Histogram extends Component {
originalClassName: 'bar'
})
)
.style('fill', (d: any) => this.model.getFillColor(d[groupMapsTo]))
.style('fill', (d: any) => this.model.getFillColor(d[groupMapsTo], undefined, d.data))
.attr('d', (d: any) => {
const bin = get(d, 'data')

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/graphs/line.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export class Line extends Component {
originalClassName: 'line'
})
)
.style('stroke', (group: any) => this.model.getStrokeColor(group.name))
.style('stroke', (group: any) => this.model.getStrokeColor(group.name, undefined, group.data))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.style('stroke', (group: any) => this.model.getStrokeColor(group.name, undefined, group.data))
.style('stroke', (group: any) => this.model.getStrokeColor(group.name, null, group.data))

// a11y
.attr('role', Roles.GRAPHICS_SYMBOL)
.attr('aria-roledescription', 'line')
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/graphs/meter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export class Meter extends Component {
.attr('width', (d: any) => {
return d.value > domainMax ? xScale(domainMax) : d.width
})
.style('fill', (d: any) => self.model.getFillColor(d[groupMapsTo]))
.style('fill', (d: any) => self.model.getFillColor(d[groupMapsTo], undefined, d))
// a11y
.attr('role', Roles.GRAPHICS_SYMBOL)
.attr('aria-roledescription', 'value')
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/graphs/pie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class Pie extends Component {
originalClassName: 'slice'
})
)
.style('fill', (d: any) => self.model.getFillColor(d.data[groupMapsTo]))
.style('fill', (d: any) => self.model.getFillColor(d.data[groupMapsTo], undefined, d.data))
.attr('d', this.arc)

allPaths
Expand Down
13 changes: 7 additions & 6 deletions packages/core/src/components/graphs/radar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ export class Radar extends Component {
.nice(yTicksNumber)
const yTicks = yScale.ticks(yTicksNumber)

const colorScale = (group: string): string => this.model.getFillColor(group)
const colorScale = (group: string, key?: any, data?: any): string =>
this.model.getFillColor(group, key, data)

// constructs a new radial line generator
// the angle accessor returns the angle in radians with 0° at -y (12 o’clock)
Expand Down Expand Up @@ -366,9 +367,9 @@ export class Radar extends Component {
? () => `translate(${c.x}, ${c.y}) scale(${1 + Math.random() * 0.35})`
: `translate(${c.x}, ${c.y})`
)
.style('fill', (group: any) => colorScale(group.name))
.style('fill', (group: any) => colorScale(group.name, undefined, group.data))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.style('fill', (group: any) => colorScale(group.name, undefined, group.data))
.style('fill', (group: any) => colorScale(group.name, null, group.data))

.style('fill-opacity', radarConfigs.opacity.selected)
.style('stroke', (group: any) => colorScale(group.name))
.style('stroke', (group: any) => colorScale(group.name, undefined, group.data))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.style('stroke', (group: any) => colorScale(group.name, undefined, group.data))
.style('stroke', (group: any) => colorScale(group.name, null, group.data))

.call((selection: any) => {
const selectionUpdate = selection.transition().call((t: any) =>
this.services.transitions.setupTransition({
Expand Down Expand Up @@ -397,8 +398,8 @@ export class Radar extends Component {
originalClassName: 'blob'
})
)
.style('fill', (group: any) => colorScale(group.name))
.style('stroke', (group: any) => colorScale(group.name))
.style('fill', (group: any) => colorScale(group.name, undefined, group.data))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.style('fill', (group: any) => colorScale(group.name, undefined, group.data))
.style('fill', (group: any) => colorScale(group.name, null, group.data))

.style('stroke', (group: any) => colorScale(group.name, undefined, group.data))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.style('stroke', (group: any) => colorScale(group.name, undefined, group.data))
.style('stroke', (group: any) => colorScale(group.name, null, group.data))

update.call((selection: any) =>
selection
.transition()
Expand Down Expand Up @@ -717,7 +718,7 @@ export class Radar extends Component {
.map((d: any) => ({
label: d[groupMapsTo],
value: d[valueMapsTo],
color: self.model.getFillColor(d[groupMapsTo]),
color: self.model.getFillColor(d[groupMapsTo], undefined, d),
class: self.model.getColorClassName({
classNameTypes: [ColorClassNameTypes.TOOLTIP],
dataGroupName: d[groupMapsTo]
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/components/graphs/treemap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export class Treemap extends Component {
.attr('height', (d: any) => d.y1 - d.y0)
.style('fill', (d: any) => {
while (d.depth > 1) d = d.parent
return this.model.getFillColor(d.data.name)
return this.model.getFillColor(d.data.name, undefined, d.data)
})

// Update all clip paths
Expand Down Expand Up @@ -252,7 +252,7 @@ export class Treemap extends Component {
})
)
.style('fill', (d: any) => {
const customColor = self.model.getFillColor(d.parent.data.name)
const customColor = self.model.getFillColor(d.parent.data.name, undefined, d.data)
if (customColor) {
fillColor = customColor
}
Expand Down Expand Up @@ -320,7 +320,7 @@ export class Treemap extends Component {
name: 'graph_element_mouseout_fill_update'
})
)
.style('fill', (d: any) => self.model.getFillColor(d.parent.data.name))
.style('fill', (d: any) => self.model.getFillColor(d.parent.data.name, undefined, d.data))

// Dispatch mouse event
self.services.events.dispatchEvent(Events.Treemap.LEAF_MOUSEOUT, {
Expand Down
Loading