Macro runs on wrong workbook on close

[Cumberland]

Hi all,

I have a macro on a particular workbook that runs when the workbook closes.

This runs fine when I close the workbook itself. If I happen to have other workbooks open and then I close EXCEL rather than an individual workbook, my macro that runs on close always runs when it is not the active workbook.

To put it another way, say that my 'run on close' macro is workbook 1. I also have workbooks 2 and 3 open as well. If workbook 3 is the active workbook and I choose to close Excel (and thus close all three workbooks), my 'run on close' macro runs straight away and, worse still, will run it on workbook 3 (my active workbook).

How can I stop the macro running until it is the active workbook in the closing chain, so that it only runs on the correct workbook? (workbook 1 in my example)

Thanks for any help.

[Phil_V]

Your macro must not be specifying which workbook to operate on, so Excel is defaulting to using the ActiveWorkbook.
If you paste the code of your BeforeClose macro changes can be adjusted to make sure it runs where it is supposed to
If you are happy to edit the code yourself you need to make sure everything is reference to 'ThisWorkbook'.

[Cumberland]

Thanks for your response.

I've tried a workaround thus:

Private Sub Workbook_BeforeClose(Cancel As Boolean)

    If ActiveWorkbook.Name = "Workbook1.xls" Then

        'closure code
        .
        .

    End If

End Sub

The problem with this is that, whilst it won't run the closure code if the active workbook isn't Workbook 1, but it won't run the macro when it does become the active workbook, because the macro has already been run when it checked for the correct workbook the first time around (sorry if that sounds confusing).

So, I've found a way to stop it running if the active workbook is the wrong one, but then it won't run again when the active workbook then becomes the right one.

[Phil_V]

You are correct, that is the drawback of coding it that way.
As I said before, if you post your 'BeforeClose' code then we can offer advice on how to alter it to make it work on the correct workbook.

[Cumberland]

Here is the actual code - all it actually does is run another macro. The 'Sort' macro just sorts a range of cells into alphabetical order before saving and closing. There are occasions where the workbook is opened as read only and the 'save changes' dialogue is not wanted, hence the check on the first line:

Private Sub Workbook_BeforeClose(Cancel As Boolean)

        If ActiveWorkbook.ReadOnly = False Then
            Application.Run ("Sort")
            ActiveWorkbook.Save
        Else
            Me.Saved = True
        End If

End Sub

[Phil_V]

ok, sorry, I should have been clearer, it is the code that runs on the wrong workbook that we need to see, so in this case I would need to see the 'Sort' macro.

[Cumberland]

OK, here it is:

Sub Sort()

    Application.ScreenUpdating = False
    Sheets("Summary of tasks").Select 'Select correct sheet
    NumRows = Application.WorksheetFunction.CountA(Range("B4:B65536")) 'Calculate no of tasks
    
'If no rows to sort, then report error and exit macro
    If NumRows = 0 Then
        MsgBox ("No rows to sort!")
        Range("A4").Select
        Exit Sub
    End If

'Convert any rogue formulas to values
    Range("D4:D" & NumRows + 3).Select
    Selection.Copy
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks:=False, Transpose:=False

'Sort and format rows and exit
    Range("A4:D" & NumRows + 3).Select
    Selection.Sort Key1:=Range("B4"), Order1:=xlAscending, Header:=xlNo, OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom
    Selection.Sort Key1:=Range("D4"), Order1:=xlAscending, Header:=xlNo, OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom
    Selection.Borders(xlDiagonalDown).LineStyle = xlNone
    Selection.Borders(xlDiagonalUp).LineStyle = xlNone
    With Selection.Borders(xlEdgeLeft)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With Selection.Borders(xlEdgeTop)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With Selection.Borders(xlEdgeBottom)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With Selection.Borders(xlEdgeRight)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With Selection.Borders(xlInsideVertical)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With Selection.Borders(xlInsideHorizontal)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    
    Application.ScreenUpdating = True
    Range("A4").Select
End Sub

[Phil_V]

Try the following instead.

Note that at the beginning I create a reference to to the worksheet in ThisWorkbook. The object 'ThisWorkbook' always represents the workbook that the macro is stored in.
I have also done away with a lot of the select statements as they are unneccasary. You do not need to select a range, and then refer to the selection, you can just refer directly to the range.
For example;

Range("D4:D" & NumRows + 3).Select
    Selection.Copy

becomes:

Range("D4:D" & NumRows + 3).Copy

Here is the code:

Sub Sort()
Dim ws As Worksheet
Application.ScreenUpdating = False

Set ws = ThisWorkbook.Sheets("Summary of tasks") 'Reference the correct sheet in THIS workbook

NumRows = Application.WorksheetFunction.CountA(ws.Range("B4:B65536")) 'Calculate no of tasks

'If no rows to sort, then report error and exit macro
If NumRows = 0 Then
    MsgBox ("No rows to sort!")
    ws.Range("A4").Select
    Exit Sub
End If

'Convert any rogue formulas to values
ws.Range("D4:D" & NumRows + 3).Copy
ws.Range("D4:D" & NumRows + 3).PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks:=False, Transpose:=False

'Sort and format rows and exit
With ws.Range("A4:D" & NumRows + 3)
    .Sort Key1:=ws.Range("B4"), Order1:=xlAscending, Header:=xlNo, OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom
    .Sort Key1:=ws.Range("D4"), Order1:=xlAscending, Header:=xlNo, OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom
    .Borders(xlDiagonalDown).LineStyle = xlNone
    .Borders(xlDiagonalUp).LineStyle = xlNone
    With .Borders(xlEdgeLeft)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With .Borders(xlEdgeTop)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With .Borders(xlEdgeBottom)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With .Borders(xlEdgeRight)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With .Borders(xlInsideVertical)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
    With .Borders(xlInsideHorizontal)
        .LineStyle = xlContinuous
        .Weight = xlThin
        .ColorIndex = 15
    End With
End With
Application.ScreenUpdating = True
ws.Range("A4").Select
End Sub

Incidently I have highlighted the sort lines as I am unsure what you are trying to achieve there?
At the moment it sorts into Ascending order by B4, then it takes that data and resorts it again but this time into Ascending order by D4, as in two SEPERATE sorts where the second one will destroy the results of the first one.
Were you actually wanting to sort by B4, and then have those sorted results sorted by D4 as a secondary sort?

[Cumberland]

Thanks for your efforts - I'll get to your code refinements in a moment.

Firstly, I've learnt that the sheet doesn't have to be visible on the screen to carry out certain functions via VBA. Interesting!

However, the macro still fails if this is not the active workbook at the time of closing Excel. All works fine until the sort statement and fails with runtime error 1004 (sort reference is not valid). It works fine if this IS the active workbook at the time of closing Excel. You will probably know the reason why this happens better than I do.

However, through experimenting, I have discovered that my original problem can be solved really easily by simply inserting the following line as the first statement in Workbook_BeforeClose:

ThisWorkbook.Activate

So that, when closing Excel, it activates the correct workbook straight away! Eureka! It now works like a dream.

Now onto your code refinements - I have learnt a couple of tricks from you, so a big thanks - I actually wrote the macro about three years ago when in my infancy of VBA programming and refining the code has simply not been high on my priority list, although the two sort statements that you highlighted should, of course, be:

Selection.Sort Key1:=Range("D4"), Order1:=xlAscending, Key2:=Range("B4") _
     , Order2:=xlAscending, Header:=xlNo, OrderCustom:=1, MatchCase:=False, Orientation:=xlTopToBottom

Thanks again for your help!

[Cumberland]

Can't believe how easy this was to solve in the end - and glad that I solved it myself!