Skip to content

This closes #2108, Add helper to auto scale sheet on page by single, columns and rows options#2109

Merged
xuri merged 1 commit intoqax-os:masterfrom
moisespsena-go:task_2108
Mar 23, 2025
Merged

This closes #2108, Add helper to auto scale sheet on page by single, columns and rows options#2109
xuri merged 1 commit intoqax-os:masterfrom
moisespsena-go:task_2108

Conversation

@moisespsena
Copy link
Contributor

  • Add new SheetAutoScalingPageMode data type
  • Add new consts SheetFitOnPage, SheetFitAllColumnsOnPage, SheetFitAllRowsOnPage
  • Add new function SetSheetAutoScalingPageByMode
  • Update unit tests

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@xuri xuri added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 22, 2025
Copy link
Member

@xuri xuri left a comment

Choose a reason for hiding this comment

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

Thanks for your PR. If the user uses the SetSheetAutoScalingPageByMode function together with the SetPageLayout or SetSheetProps functions, the presets in the SetSheetAutoScalingPageByMode function may be modified, potentially leading to unexpected results and confusion.

Add documentation describing how to combine the existing SetPageLayout and SetSheetProps functions to achieve the 3 scaling options mentioned in issue #2108, would be preferred approach.

Due to the principle of minimum availability, there is no plan to introduce this feature to keep the library core and easy.

Copy link
Member

@xuri xuri left a comment

Choose a reason for hiding this comment

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

I suggest revert changes in the commit b2a4725 and comments for the SetSheetProps function like this:

// SetSheetProps provides a function to set worksheet properties. There 4 kinds
// of presets "Custom Scaling Options" in the spreadsheet applications, if you
// need to set those kind of scaling options, please using the "SetSheetProps"
// and "SetPageLayout" functions to approach these 4 scaling options:
//
// 1. No Scaling (Print sheets at their actual size):
//
//	disable := false
//	if err := f.SetSheetProps("Sheet1", &excelize.SheetPropsOptions{
//	    FitToPage: &disable,
//	}); err != nil {
//	    fmt.Println(err)
//	}
//
// 2. Fit Sheet on One Page (Shrink the printout so that it fits on one page):
//
//	enable := true
//	if err := f.SetSheetProps("Sheet1", &excelize.SheetPropsOptions{
//	    FitToPage: &enable,
//	}); err != nil {
//	    fmt.Println(err)
//	}
//
// 3. Fit All Columns on One Page (Shrink the printout so that it is one page
// wide):
//
//	enable, zero := true, 0
//	if err := f.SetSheetProps("Sheet1", &excelize.SheetPropsOptions{
//	    FitToPage: &enable,
//	}); err != nil {
//	    fmt.Println(err)
//	}
//	if err := f.SetPageLayout("Sheet1", &excelize.PageLayoutOptions{
//	    FitToHeight: &zero,
//	}); err != nil {
//	    fmt.Println(err)
//	}
//
// 4. Fit All Rows on One Page (Shrink the printout so that it is one page
// high):
//
//	enable, zero := true, 0
//	if err := f.SetSheetProps("Sheet1", &excelize.SheetPropsOptions{
//	    FitToPage: &enable,
//	}); err != nil {
//	    fmt.Println(err)
//	}
//	if err := f.SetPageLayout("Sheet1", &excelize.PageLayoutOptions{
//	    FitToWidth: &zero,
//	}); err != nil {
//	    fmt.Println(err)
//	}

…) No Scaling; 2) Fit Sheet on One Page; 3) Fit All Columns on One Page; 4) Fit All Rows on One Page
@moisespsena
Copy link
Contributor Author

@xuri Thank you for suggestion. I aplly it.

Copy link
Member

@xuri xuri left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution.

@codecov
Copy link

codecov bot commented Mar 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.20%. Comparing base (d399e7b) to head (39bc89a).
Report is 25 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2109   +/-   ##
=======================================
  Coverage   99.20%   99.20%           
=======================================
  Files          32       32           
  Lines       30104    30104           
=======================================
  Hits        29866    29866           
  Misses        158      158           
  Partials       80       80           
Flag Coverage Δ
unittests 99.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xuri xuri merged commit e9d27c7 into qax-os:master Mar 23, 2025
17 checks passed
@xuri xuri added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 23, 2025
@xuri xuri moved this to Miscellaneous in Excelize v2.9.1 Jun 4, 2025
@xuri xuri added the kind/documentation Improvements or additions to documentation label Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/documentation Improvements or additions to documentation size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

No open projects
Status: Miscellaneous

Development

Successfully merging this pull request may close these issues.

2 participants

Comments