korbit-ai[bot] commented on code in PR #36085:
URL: https://github.com/apache/superset/pull/36085#discussion_r2518020914
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -525,7 +525,7 @@ export default function transformProps(
nameGap: convertInteger(xAxisTitleMargin),
nameLocation: 'middle',
axisLabel: {
- hideOverlap: true,
+ hideOverlap: xAxisType !== AxisType.Time,
Review Comment:
### Time axis label overlap not prevented <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The conditional logic for hideOverlap may cause label overlap issues for
time-based charts where labels could still overlap despite being disabled.
###### Why this matters
When xAxisType is AxisType.Time, hideOverlap will be set to false, which
means overlapping labels will be displayed. This could result in unreadable or
cluttered time axis labels, especially when there are many data points or long
time labels that naturally overlap.
###### Suggested change ∙ *Feature Preview*
Consider the specific use case requirements. If time labels should never
overlap, keep `hideOverlap: true` for all axis types. If time labels need to
show all values even when overlapping, consider adding additional logic to
handle label formatting or rotation:
```typescript
hideOverlap: xAxisType !== AxisType.Time ? true : shouldShowAllTimeLabels,
```
Or implement proper label spacing/rotation for time axes to prevent overlap
while showing all labels.
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/54cc699e-819b-4f9f-b837-66b4cc145804/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/54cc699e-819b-4f9f-b837-66b4cc145804?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/54cc699e-819b-4f9f-b837-66b4cc145804?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/54cc699e-819b-4f9f-b837-66b4cc145804?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/54cc699e-819b-4f9f-b837-66b4cc145804)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:4c5daba5-1cb8-4ed3-be0c-692c944e76e8 -->
[](4c5daba5-1cb8-4ed3-be0c-692c944e76e8)
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -525,7 +525,7 @@ export default function transformProps(
nameGap: convertInteger(xAxisTitleMargin),
nameLocation: 'middle',
axisLabel: {
- hideOverlap: true,
+ hideOverlap: xAxisType !== AxisType.Time,
Review Comment:
### Unnecessary computation in hideOverlap property <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The hideOverlap property is being computed on every render by comparing
xAxisType to AxisType.Time, which is an unnecessary computation since these
values don't change during the component lifecycle.
###### Why this matters
This creates a minor performance overhead as the comparison is executed on
every function call, even though the result will be the same for a given
xAxisType value throughout the component's lifecycle.
###### Suggested change ∙ *Feature Preview*
Pre-compute the boolean value and store it in a variable before the xAxis
object definition:
```typescript
const shouldHideOverlap = xAxisType !== AxisType.Time;
let xAxis: any = {
type: xAxisType,
name: xAxisTitle,
nameGap: convertInteger(xAxisTitleMargin),
nameLocation: 'middle',
axisLabel: {
hideOverlap: shouldHideOverlap,
formatter: xAxisFormatter,
rotate: xAxisLabelRotation,
interval: xAxisLabelInterval,
},
// ... rest of config
};
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/80a62f0a-48fc-4901-aaae-8e0f2a117f79/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/80a62f0a-48fc-4901-aaae-8e0f2a117f79?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/80a62f0a-48fc-4901-aaae-8e0f2a117f79?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/80a62f0a-48fc-4901-aaae-8e0f2a117f79?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/80a62f0a-48fc-4901-aaae-8e0f2a117f79)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:1efbfc92-d7d1-4425-86e2-8d04cda9fc48 -->
[](1efbfc92-d7d1-4425-86e2-8d04cda9fc48)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
