Skip to content

[Excel] (Chart) Add leader lines snippet #992

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alison-mk
Copy link
Contributor

This PR is in draft mode because the enableLeaderLines method isn't working properly yet -- it's adding leader lines, which shouldn't happen until the enableLeaderLines method.

@alison-mk alison-mk requested a review from Copilot July 18, 2025 17:34
Copilot

This comment was marked as outdated.

@alison-mk alison-mk requested a review from Copilot July 18, 2025 18:38
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new Excel chart leader lines snippet that demonstrates how to show, hide, and format leader lines for chart data labels. The sample provides a complete workflow for working with leader lines in Excel charts using Office.js APIs.

  • Adds a new sample file demonstrating leader lines functionality for Excel charts
  • Implements functions to enable/disable leader lines and modify their formatting
  • Provides UI controls for testing all leader line operations

Comment on lines +55 to +57
// Load the current leader lines setting.
dataLabels.load("showLeaderLines");
await context.sync();
Copy link
Preview

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

The comment suggests loading the current setting, but the loaded value is never used in the function. Consider removing the load operation and comment, or clarify why the current value needs to be loaded.

Suggested change
// Load the current leader lines setting.
dataLabels.load("showLeaderLines");
await context.sync();

Copilot uses AI. Check for mistakes.

Comment on lines +55 to +60
// Load the current leader lines setting.
dataLabels.load("showLeaderLines");
await context.sync();

// Enable leader lines.
dataLabels.showLeaderLines = true;
Copy link
Preview

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

The showLeaderLines property is loaded but never accessed before being set to true. This load operation appears unnecessary and could be removed for better performance.

Suggested change
// Load the current leader lines setting.
dataLabels.load("showLeaderLines");
await context.sync();
// Enable leader lines.
dataLabels.showLeaderLines = true;
// Enable leader lines.
dataLabels.showLeaderLines = true;
await context.sync();

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant