Skip to content
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

Fix Issue 1344 with Freeze Top Row #1356

Closed
wants to merge 3 commits into from

Conversation

thkn-hofa
Copy link
Contributor

As described by @mscreations:

if ($Title) {
    $ws.View.FreezePanes(2, 1)
    Write-Verbose -Message "Froze title and header rows"
}

Should be

if ($Title) {
    $ws.View.FreezePanes(3, 1)
    Write-Verbose -Message "Froze title and header rows"
}

FreezetopRowFirstColumn does the same for freezing the top row with a title:

#Allow single switch or two seperate ones.
if ($FreezeTopRowFirstColumn -or ($FreezeTopRow -and $FreezeFirstColumn)) {
    if ($Title) {
        $ws.View.FreezePanes(3, 2) ### THIS LINE RIGHT HERE AS EXAMPLE
        Write-Verbose -Message "Froze title and header rows and first column"
    }
    else {
        $ws.View.FreezePanes(2, 2)
        Write-Verbose -Message "Froze top row and first column"
    }
}
elseif ($FreezeTopRow) {
    if ($Title) {
        $ws.View.FreezePanes(3, 1) ### THIS LINE RIGHT HERE WHAT NEEDED TO BE FIXED
        Write-Verbose -Message "Froze title and header rows"
    }
    else {
        $ws.View.FreezePanes(2, 1)
        Write-Verbose -Message "Froze top row"
    }
}

Fixes issue #1344

@dfinke
Copy link
Owner

dfinke commented Dec 25, 2022

@thkn-hofa Thanks for the PR. Are there tests for these?

@thkn-hofa
Copy link
Contributor Author

No, there weren't any before and didn't feel like writing checks just for this bugfix

@dfinke
Copy link
Owner

dfinke commented Dec 26, 2022

Yup, one of the reasons I probably won't merge. Tests were introduced late. Never knew this would catch on. Making some changes, like this is time for the introduction of tests. Lots of effort though.

@thkn-hofa
Copy link
Contributor Author

Just saw there is a test for freezetoprowandfirstcolumn. I'll add a test for first row and first column when I get the chance

@thkn-hofa
Copy link
Contributor Author

thkn-hofa commented Dec 26, 2022

Tests for feezing panes

  • Freeze top row and first column without title
  • Freeze top row and first column with title
  • Freeze top row without title
  • Freeze top row with title
  • Freeze first Column

@dfinke
Copy link
Owner

dfinke commented Dec 26, 2022

If I can get time, I'll look at the height issue, see what tests may exist, and augment.

@dfinke
Copy link
Owner

dfinke commented Dec 27, 2022

Tried the below in a test. Didn't get the same results from -FreezeTopRow as the -FreezePane 3, 1.
If I understand the issue, it should freeze the first and second row. Title and "header".

$path = "$pwd\testFreeze.xlsx"

Remove-Item $path -ErrorAction SilentlyContinue
        
$data = ConvertFrom-Csv @"
        Region,State,Units,Price
        West,Texas,927,923.71
        North,Tennessee,466,770.67
        East,Florida,520,458.68
        East,Maine,828,661.24
        West,Virginia,465,053.58
        North,Missouri,436,235.67
        South,Kansas,214,992.47
        North,North Dakota,789,640.72
        South,Delaware,712,508.55
"@

Export-Excel -InputObject $data -Path $path -TableName 'TestTable' -WorksheetName 'TestSheet' -AutoSize -TableStyle Medium2 -Title 'Test Title' -TitleBold -TitleSize 18 -Show -FreezeTopRow # -FreezePane 3, 1

@thkn-hofa
Copy link
Contributor Author

thkn-hofa commented Dec 27, 2022

If I understand the issue, it should freeze the first and second row. Title and "header".

That's what I understood, too

I get the expected results when running your snippet:

image

Are you sure you loaded this branch in your console?

PS C:\source\repos\ImportExcel> remove-module importexcel; import-module .\ImportExcel.psd1

Check the loaded Export-Excel function:

((Get-Command Export-Excel).Definition -Split "[\n]")[494..503]

Should yield

elseif ($FreezeTopRow) {
    if ($Title) {
        $ws.View.FreezePanes(3, 1)
        Write-Verbose -Message "Froze title and header rows"
    }
    else {
        $ws.View.FreezePanes(2, 1)
        Write-Verbose -Message "Froze top row"
    }
}

and NOT $ws.View.FreezePanes(2, 1) in the if ($Title) { ... } codeblock

@dfinke
Copy link
Owner

dfinke commented Dec 28, 2022

Thanks for checking. I pulled down your PR, maybe I didn't do a -force on the ipmo

@dfinke
Copy link
Owner

dfinke commented Dec 28, 2022

Closing this, resolved conflicts here: #1361

@dfinke dfinke closed this Dec 28, 2022
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.

None yet

2 participants