Skip to content

fix(Axistip): add supports the formatter and fix the tips display abnormally #248

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix(Axistip): add supports the formatter and fix the tips display abnormally #248

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 12 additions & 2 deletions src/feature/axistip/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@ import { isArray, isBoolean } from '../../util/type';
const distanceX = 16; // 鼠标与气泡框之间的左右偏移量
const distanceY = 24; // 鼠标与气泡框之间的上偏移量
const axisType = ['xAxis', 'yAxis'];
let textFormatter = {};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider scoping textFormatter to avoid state persistence between calls

The textFormatter variable is declared at module scope using let, which means its state will persist between multiple calls to the axistip function. This could lead to unexpected behavior if formatters from previous chart instances are applied to new charts.

-let textFormatter = {};

Move the declaration inside the axistip function to reset it on each call:

function axistip(echartsDom, echartsIns, eChartOption, axistip) {
  if (!axistip) return;
+  let textFormatter = {};
  
  if (isBoolean(axistip)) {
    axistip = {};
    axisType.forEach(item => {
      axistip[item] = true;
    })
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let textFormatter = {};
// --- Remove this module‑scope line:
//- let textFormatter = {};
// Move into the function to reset on each call:
function axistip(echartsDom, echartsIns, eChartOption, axistip) {
if (!axistip) return;
let textFormatter = {};
if (isBoolean(axistip)) {
axistip = {};
axisType.forEach(item => {
axistip[item] = true;
});
}
// …rest of axistip implementation that uses textFormatter…
}


// 设置 TriggerEvent
function setAxisTriggerEvent(eChartOption, type) {
if (!eChartOption[type]) return;
if (isArray(eChartOption[type])) {
eChartOption[type].forEach((subitem) => {
textFormatter[type] = undefined;
if(subitem.axisLabel?.formatter){
textFormatter[type] = subitem.axisLabel.formatter
}
Comment on lines +25 to +28
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add formatter handling for non-array axis configuration

The formatter is only extracted when eChartOption[type] is an array, but not when it's a single object. This is inconsistent with how triggerEvent is handled.

} else {
+  textFormatter[type] = undefined;
+  if(eChartOption[type].axisLabel?.formatter){
+    textFormatter[type] = eChartOption[type].axisLabel.formatter
+  }
  eChartOption[type].triggerEvent = true;
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
textFormatter[type] = undefined;
if(subitem.axisLabel?.formatter){
textFormatter[type] = subitem.axisLabel.formatter
}
} else {
textFormatter[type] = undefined;
if (eChartOption[type].axisLabel?.formatter) {
textFormatter[type] = eChartOption[type].axisLabel.formatter
}
eChartOption[type].triggerEvent = true;
}

subitem.triggerEvent = true;
})
} else {
Expand Down Expand Up @@ -78,6 +83,7 @@ function axistip(echartsDom, echartsIns, eChartOption, axistip) {
})
}
Object.keys(axistip).forEach(item => {
if(!axistip[item]) return;
setAxisTriggerEvent(eChartOption, item);
})
// 气泡容器
Expand All @@ -86,8 +92,12 @@ function axistip(echartsDom, echartsIns, eChartOption, axistip) {
tipContainer.style.display = 'inline-block';
tipContainer.style.opacity = '0';
echartsIns.on('mousemove', (param) => {
tipContainer.textContent = param.value;

let type = param.componentType;
if(param.name){
tipContainer.textContent = param.name;
}else{
tipContainer.textContent = textFormatter[type] ? textFormatter[type](param.value) : param.value;
}
Comment on lines +95 to +100
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for formatter functions

The current implementation directly calls the formatter function without any error handling. If the formatter throws an error, it could break the entire chart's functionality.

-tipContainer.textContent = textFormatter[type] ? textFormatter[type](param.value) : param.value;
+try {
+  tipContainer.textContent = textFormatter[type] ? textFormatter[type](param.value) : param.value;
+} catch (error) {
+  console.error('Error applying axis formatter:', error);
+  tipContainer.textContent = param.value;
+}

Also, maintain consistent use of spaces in conditional expressions:

-if(param.name){
+if (param.name) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let type = param.componentType;
if(param.name){
tipContainer.textContent = param.name;
}else{
tipContainer.textContent = textFormatter[type] ? textFormatter[type](param.value) : param.value;
}
let type = param.componentType;
if (param.name) {
tipContainer.textContent = param.name;
} else {
try {
tipContainer.textContent = textFormatter[type]
? textFormatter[type](param.value)
: param.value;
} catch (error) {
console.error('Error applying axis formatter:', error);
tipContainer.textContent = param.value;
}
}

if(axisType.indexOf(param.componentType) !== -1) {
setPosition(tipContainer, echartsDom, param);
}
Expand Down