-
Notifications
You must be signed in to change notification settings - Fork 270
[Excel] Update low engagement articles #5374
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
Conversation
Learn Build status updates of commit d8053d5: ✅ Validation status: passed
For more details, please refer to the build report. |
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 issues. Other issues are also a high priority. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
There was a problem hiding this 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 updates several Excel JavaScript API articles to improve engagement by enhancing readability, adding structured guidance, and including actionable patterns. The updates focus on performance optimization, range operations, and data validation topics.
- Restructured content with quick reference sections and concrete implementation strategies
- Added comparison tables, best practices guides, and troubleshooting patterns
- Enhanced code examples with clearer explanations and next steps sections
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
docs/excel/performance.md | Major restructure with quick improvements section, clearer guidance on batching and payload limits |
docs/excel/excel-add-ins-ranges-special-cells.md | Added quick reference table and enhanced explanations of getSpecialCells methods |
docs/excel/excel-add-ins-ranges-precedents-dependents.md | Added key points section and best practices table for precedents/dependents |
docs/excel/excel-add-ins-ranges-large.md | Complete rewrite with structured guidance on handling large ranges and resource limits |
docs/excel/excel-add-ins-multiple-ranges.md | Enhanced with key points and RangeAreas combination patterns |
docs/excel/excel-add-ins-delay-in-cell-edit.md | Expanded with behavior comparison table and clearer guidance |
docs/excel/excel-add-ins-data-validation.md | Improved structure with clearer section headers and enhanced code explanations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 issues. Other issues are also a high priority. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
Learn Build status updates of commit a4b4076: ✅ Validation status: passed
For more details, please refer to the build report. |
…fficeDev/office-js-docs-pr into alison-mk-fhl-low-engagement-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
docs/excel/excel-add-ins-ranges-precedents-dependents.md:1
- Non-standard hyphen character used. Should use a standard hyphen (-) instead of the en-dash (‑) in 'non‑contiguous'.
---
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 issues. Other issues are also a high priority. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
Learn Build status updates of commit a79229c: ✅ Validation status: passed
For more details, please refer to the build report. |
Co-authored-by: Copilot <[email protected]>
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 issues. Other issues are also a high priority. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
Learn Build status updates of commit 8b3ec4f: ✅ Validation status: passed
For more details, please refer to the build report. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A partial review, but some general thoughts for the whole PR amongst the comments.
When you import large datasets directly into a [Table](/javascript/api/excel/excel.table), such as repeatedly calling `TableRowCollection.add()`, performance can degrade. Instead, take the following approach: | ||
|
||
1. Write the entire 2D array to a range with `range.values`. | ||
2. Create the table over that populated range (`worksheet.tables.add()`). | ||
|
||
For existing tables, set values on `table.getDataBodyRange()` in bulk. The table expands automatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this entire tip is a relic from ExcelApi 1.1. You can add multiple rows at once as of 1.4, which doesn't seem less efficient than the tips here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think the entire "Importing data into tables" section can be removed from this Performance article?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's my understanding. Might be worth asking someone like Joe, but don't let that block your check-in.
Co-authored-by: Alex Jerabek <[email protected]>
Learn Build status updates of commit 038d46d: ✅ Validation status: passed
For more details, please refer to the build report. |
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 issues. Other issues are also a high priority. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing so quickly and providing helpful feedback! A few questions in response.
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 issues. Other issues are also a high priority. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
Learn Build status updates of commit dcccd1c: ✅ Validation status: passed
For more details, please refer to the build report. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up questions or responses for all of the remaining open discussions. Thanks for the great feedback!
## Create RangeAreas | ||
|
||
You can create `RangeAreas` object in two basic ways: | ||
You can create a `RangeAreas` object in two main ways: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This article has an existing section about using getSpecialCells (Get special cells from multiple ranges) and lists the getSpecialCells APIs in the methods list earlier in the article. It seems like there are many ways to create RangeAreas -- as illustrated by the methods list -- and this section is simply describing how to create RangeAreas as a broader concept. Perhaps I'm misunderstanding? If so, this is a longer conversation that we could have offline. I suspect that it doesn't block merging this PR, let me know if otherwise.
When you import large datasets directly into a [Table](/javascript/api/excel/excel.table), such as repeatedly calling `TableRowCollection.add()`, performance can degrade. Instead, take the following approach: | ||
|
||
1. Write the entire 2D array to a range with `range.values`. | ||
2. Create the table over that populated range (`worksheet.tables.add()`). | ||
|
||
For existing tables, set values on `table.getDataBodyRange()` in bulk. The table expands automatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think the entire "Importing data into tables" section can be removed from this Performance article?
## Create RangeAreas | ||
|
||
You can create `RangeAreas` object in two basic ways: | ||
You can create a `RangeAreas` object in two main ways: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment is more specific. The next two lines are Worksheet.getRanges and Worksheet.getSelectedRanges, but there are more ways to create RangeAreas (and arguably better ones). I think these were the two ways before more functions were added in a later API set.
I think this section could either be rewritten to include all the ways to generate a RangeAreas (or at least add getSpecialCells and dependants/precedents), or be changed to a section about using Worksheet.getRanges and add another section about getSelectedRanges (that way, it's organized by "getting RangeAreas through string addresses", "getting RangeAreas through user selection", and "getting RangeAreas through cell criteria").
When you import large datasets directly into a [Table](/javascript/api/excel/excel.table), such as repeatedly calling `TableRowCollection.add()`, performance can degrade. Instead, take the following approach: | ||
|
||
1. Write the entire 2D array to a range with `range.values`. | ||
2. Create the table over that populated range (`worksheet.tables.add()`). | ||
|
||
For existing tables, set values on `table.getDataBodyRange()` in bulk. The table expands automatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's my understanding. Might be worth asking someone like Joe, but don't let that block your check-in.
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 issues. Other issues are also a high priority. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
Learn Build status updates of commit 8012d1c: ✅ Validation status: passed
For more details, please refer to the build report. |
This PR updates Excel articles flagged for low engagement, using suggestions from GitHub Copilot that have then been reviewed and revised by a human.
Related PRs:
#5364
#5378