Solved: two routines under one event, but they don't work?

[andrewvanmar]

I copied the below code from an example to add multiple routines under a single event: but this one doesn't work (unlike others I made), maybe it's because it's in the workbook object instead of a sheet object?

The error I get is: Compile error: variable not defined.

It points at the word "target"

The code is entered in "this workbook"



Private Sub Workbook_Open()

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc10 Target
eventproc11 Target


Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc10()
End Sub
Run "ShowAll"
End Sub
'part of the hiding worksheets code
Private Sub eventproc11()
Call disable_menu_items
End Sub
'This code is to prevent mailing from the workbook

This one does work for example, it's on a specific worksheet.

Private Sub Worksheet_Change(ByVal Target As Range)

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc4 Target
eventproc5 Target

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc4(ByVal Target As Range)
If Not Intersect(Range("g2"), Target) Is Nothing Then
If IsDate(Target) Then
If Target.Value < Date + 1 Then
MsgBox "The date must be tomorrow or later."
Target.ClearContents
End If
End If
End If
End Sub
Private Sub eventproc5(ByVal Target As Range)
If Not Intersect(Range("G2"), Target) Is Nothing Then

'If Target.Value < Date Then
'If Target.Interior.ColorIndex = 3 Then
Target.Interior.ColorIndex = 36
'End If
'End If
End If
End Sub

[Bob Phillips]

The cell(s) being processed is passed as a parameter to the worksheet events, which you test in Target.

Workbook_Open doesn't have a Target argument, so you can't use it. You will need to specify the cells you want to work upon. But as the called subs don't have a Range argument, it seems superfluous.

Also best to remove the End Sub before the call to other procs.

[andrewvanmar]

is that the same case for Workbook_BeforeSave, Workbook_BeforeClose, Workbook_Deactivate and Workbook_WindowActivate ? (they don't seem to work either)

I'm not sure from your explanation how to set up multiple routines from one event then.

could you show me from the above example?

[rory]

You just need:
Private Sub Workbook_Open()

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc10
eventproc11


Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub


Regards,
Rory
PS All event procedures have set declarations which cannot be altered. You can find out what they are by using the dropdowns at the top of the main code window in the VB Editor. Different events provide different arguments that you can work with. For example, all Workbook_Before... events procide a Cancel argument that you can use to cancel the event.

[andrewvanmar]

Hmm, it doesn't whinge about the word target now, but it does point at the ?ventproc10 and say "Argument not optional".
What does that mean?

[rory]

Interesting choice of phrase....
It means eventproc10 is expecting an argument passed to it, or something it runs is expecting one. I'd guess the latter as what you posted did not require an argument. (I assume you tidied up the End Sub issue?)
Regards,
Rory

[andrewvanmar]

Private Sub Workbook_Open()



Application.ScreenUpdating = False
Application.EnableEvents = False


eventproc10 Target
eventproc11 Target



Application.EnableEvents = True
Application.ScreenUpdating = True


End Sub


Private Sub eventproc10()


Run "ShowAll"


'part of the hiding worksheets code
Private Sub eventproc11()
Call disable_menu_items
End Sub
'This code is to prevent mailing from the workbook


You mean like this?
(btw: i meant whinge (i looked up the other, not what I meant at all. haha. editing that out)

[rory]

:)
No like this:


Private Sub Workbook_Open()





Application.ScreenUpdating = False
Application.EnableEvents = False




eventproc10
eventproc11





Application.EnableEvents = True
Application.ScreenUpdating = True




End Sub




Private Sub eventproc10()




Run "ShowAll"




'part of the hiding worksheets code
Private Sub eventproc11()
Call disable_menu_items
End Sub

if you still get an error, then either ShowAll or disable_menu_items expects an argument passed to it.

Regards,
Rory

[Bob Phillips]

BTW, you do know that it is APplication.Run, not Run don't you?

[rory]

It should work without specifying Application, but I would agree it is better to use it.

[andrewvanmar]

@ XLD; nope I didn't, but i'm a quick study ( at least that's what I tell myself) so now I do. :)

@ Rory: saw that I copied the "targets", dumb of me. I'm trying this right away

tested: I get an -expected end sub- after application.run "showall"

[rory]

Yep, sorry, I missed that. Just type:
End Sub
after that line.

Regards,
Rory

[andrewvanmar]

I added the end sub, and it works :)

I'm comparing the code before and after; i don't get it, what is the difference? the spacing is larger, but i'm missing something here since I can't reproduce it on the rest of the code yet

[rory]

We removed the Target argument that you were passing to the macros you called. Neither of them take an argument, and the Target variable was empty anyway!
Regards,
Rory

[andrewvanmar]

hmmm, I did that immediatly over the whole set.

would you mind looking at this one then? it's the next one i was trying to fix, but for some (different) reason it doesn't work. It also gives that Argument not optional error.

We (you actually) did something to fix that, but I don't what

Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)

Application.ScreenUpdating = False
Application.EnableEvents = False



eventproc6
eventproc7




Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc6(ByVal SaveAsUI As Boolean, Cancel As Boolean)
If IsComplete = False Then
Cancel = True
MsgBox "Cannot save, please fill in all the fields in the orderform."
Else
Cancel = False
End If
'to block saving when certain fields are not filled in
End Sub
Private Sub eventproc7()
If Cancel = True Or bIsClosing = False Then Exit Sub
Application.Run "HideAll"
End Sub
'part of the hiding code

[rory]

You could do it like this:
Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc6 Cancel
eventproc7 Cancel

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc6(ByRef Cancel As Boolean)
If iscomplete = False Then
Cancel = True
MsgBox "Cannot save, please fill in all the fields in the orderform."
Else
Cancel = False
End If
'to block saving when certain fields are not filled in
End Sub
Private Sub eventproc7(ByVal Cancel As Boolean)
If Cancel = True Or bisclosing = False Then Exit Sub
Application.Run "HideAll"
End Sub


but I wouldn't personally, I would put the code in the body of the event routine since it depends on the Cancel argument - something like:
Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)

Application.ScreenUpdating = False
Application.EnableEvents = False

If iscomplete = False Then
Cancel = True
MsgBox "Cannot save, please fill in all the fields in the orderform."
Else
Cancel = False
End If
If Cancel = False And bIsClosing = True Then Application.Run "HideAll"

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub


Regards,
Rory

[andrewvanmar]

why the changing or to and?

Aside from that, I can do this to the others i mentioned as well?
gonna try immediatly! :)

[rory]

No real reason other than it's shorter than:
If Cancel = True Or bisclosing = False Then

Else
Application.Run "HideAll"
End If


Regards,
Rory

[andrewvanmar]

Ah! see lots to learn *grin*

Let me go through the code as I rearrange things, and then post the book for you to see (if you're curious)

[rory]

As long as it's still intelligible, shorter = better in my book. Clarity and maintainability are more important than brevity, though.
I would be intrigued to see it...
Rory

[andrewvanmar]

6351

Not quite there yet.(whince)

There is a set of events supposed to trigger at some points:
-printing pdf-ng and saving are cancelled when certain fields are not filled in (works)
-the emailing commands are disabled but enabled when switching to another workbook, or when closing the workbook. (appears to work)
-When macro's are disabled everything but the manual sheet is supposed to be "very hidden" and vice versa (doesn't work yet)

anyway, have a look, probably you'll see what i'm missing immediatly :)

[andrewvanmar]

OH before I forget: the date picker needs mscal.ocx, and .netframework 2 (apparantly the stupid thing is not downwards compatible) so it could be that the datepicker will not work for you.
The pdf will most certainly not work unless you have pdfcreator installed:
http://www.pdfforge.org/products/pdfcreator/download (incase you are curious)
I'll have to have everyone here install it (along with the framework upgrade and the mscal, but ah...the rewards ;-) )

[andrewvanmar]

be back tomorrow!

[andrewvanmar]

did you see see anything out of the ordinary?

[rory]

Unfortunately there's no point looking at work (can't install anything on my machine) and didn't have time last night. Will try and have a peek tonight, but can't guarantee it!

[andrewvanmar]

No worries!
What you could do though, if you would, is have a look at the "this workbook" object (no need for installs for that functionality) the on open, on close functions seem to be not quite right the module they call to is module 4.

[rory]

What is wrong with them?

[Bob Phillips]

For what it is worth, my 2 penn'th.

The IsComplete function should be in a standard code module, not ThisWorkbook. Not a will work/won't work thing, more a style thing.

The IsComplete vraiable seems to be a bit scattered, not easy to see where and when it gets set.

You don't need Application.Run, as HideAll and ShowAll are in the same project, so a simple Call HideAll will suffice.

I would put enable_menu_items/disable_menu_items in the same module as ShowAll and HideAll.

I would also re-write



Function IsComplete() As Boolean
IsComplete = False

If Sheets("Information").[f58] = "" Then Exit Function
If Sheets("Information").[f59] = "" Then Exit Function
If Sheets("Signatures and pricing").[g2] = "" Then Exit Function


' If we got here, then all cells are filled, so OK
IsComplete = True
End Function


as



Function IsComplete() As Boolean
IsComplete = Sheets("Information").Range("F58").Value <> "" Or _
Sheets("Information").Range("F59).Value <> "" Or _
Sheets("Signatures and pricing").Range("G2").Value <> ""
End Function


But these are all relatively minor points, so is there anything specific that you think is wrong.

[andrewvanmar]

Hi XLD, thanks for the advice. I hit a few snags cleaning up though: moving the iscomplete function away from the this workbook module gives an ambiguous name error in the code that calls for it.

rewriting it, immediatly resulted in red text....

I think i'll leave that as is, and add comments to explain what goes where.

did you see any thing wrong that blocks the hiding script?

[andrewvanmar]

by the way what does the bIsClosing command do?

[Bob Phillips]

I would guess that it is a flag that tells you the workbook is closing, but as I said it was a bit scattered and I didn't track it all down. But didn't you write that bit?

[Bob Phillips]

Hi XLD, thanks for the advice. I hit a few snags cleaning up though: moving the iscomplete function away from the this workbook module gives an ambiguous name error in the code that calls for it.

rewriting it, immediatly resulted in red text....

I think i'll leave that as is, and add comments to explain what goes where.

You didn't leave theold onein as well as copying it to a module did you?

[andrewvanmar]

no, I didn't wrie it myself, the code is a frankenstein ofscavenged code i found.

I'll try again, just to be sure, but I believe I did a cut and paste.

The function is referred to by 3 othe pieces of code, two in the my workbook (the before_save and the before print) and the third in sheet 1 where the pdf-ing script uses it to check in the required fields have been filled in.

the only part that does't work is the script that hides sheets when macro's are disabled (the bIsClosing is part of that script).
I took that code from this workbook : http://www.ozgrid.com/FreeDownloads/EnableMacros.zip

if you don't allow macros to run when opening it will set all sheets but one to very hidden.( the remaining sheet contains info why the workbook is unusable untill they enable macros) but I don't understand the code very well, and especially don't understand why tere is a on deactivae event involved.
I'm at a loss there. (if I take away other pieces of code that share the events that hiding scripts use, then it works, but for some reason sharing disables it)

[andrewvanmar]

YUP, I cut and pasted, and when running the script I get the ambiguous name error. I thik, as long as this is a style issue, i'll leave it like this. Making the hide/unhide for macro enablement script is more important to me ;-)

This is all from "my workbook"
Option Explicit
Public Emptycell As Range
Public bIsClosing As Boolean
Dim wsSheet As Worksheet
'to prevent printing when certain fields are not filled in
Private Sub Workbook_BeforePrint(Cancel As Boolean)
If IsComplete = False Then
Cancel = True
MsgBox "Cannot print, please fill in all the fields in the orderform."
Else
Cancel = False
End If
End Sub
'to prevent saving when certain fields are not filled in
Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)

Application.ScreenUpdating = False
Application.EnableEvents = False

If IsComplete = False Then
Cancel = True
MsgBox "Cannot save, please fill in all the fields in the orderform."
Else
Cancel = False
End If
If Cancel = False And bIsClosing = True Then Application.Run "HideAll"
End If
Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub
'so the mail command works when switching to another workbook
Private Sub Workbook_Beforeclose(Cancel As Boolean)

Application.ScreenUpdating = False
Application.EnableEvents = False

Call enable_menu_items
bIsClosing = True


Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

'to block mail commands when opening the doc, and to show all when macros are enabled
Private Sub Workbook_Open()




Application.ScreenUpdating = False
Application.EnableEvents = False


Application.Run "ShowAll"
Call disable_menu_items



Application.EnableEvents = True
Application.ScreenUpdating = True



End Sub

'when switching to another workbook, enables mail commands and hides sheets
Private Sub Workbook_Deactivate()

Application.ScreenUpdating = False
Application.EnableEvents = False

Call enable_menu_items
If bIsClosing = True Then
Application.Run "HideAll"
End If

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

'disables the mail commands when switching back to this workbook
Private Sub Workbook_WindowActivate(ByVal Wn As Window)
Call disable_menu_items
End Sub

[andrewvanmar]

and this is from a module, the rest of the relevant script

Option Explicit
'this 'script hides the sheets except for the "manual" sheet
Sub HideAll()
Application.ScreenUpdating = False
For Each wsSheet In ThisWorkbook.Worksheets
If wsSheet.CodeName = "Sheet1" Then
wsSheet.Visible = xlSheetVisible
Else
wsSheet.Visible = xlSheetVeryHidden
End If
Next wsSheet
Application.ScreenUpdating = True
End Sub
'this script shows all sheets except for the manual one
Sub ShowAll()
bIsClosing = False
For Each wsSheet In ThisWorkbook.Worksheets
If wsSheet.CodeName <> "Sheet1" Then
wsSheet.Visible = xlSheetVisible
End If
Next wsSheet
Sheet1.Visible = xlSheetVeryHidden
End Sub


Now: if I take out the rountines that share events with this code in the my workbook, then it works, but it won't work when sharing an event with another routine.

how can I fix that?

[andrewvanmar]

I figured it out:

Because I had a canceling routing at the before save event in case certain cells hadn't geen filled it, I had to save while disabling scripts (in vb mode) because I wnt those cells to remain empty.

this ran counter to the hiding script: the hiding script is supposed to hide every time the doc is saved and closed, so that if macros are disabled, it can't run the script to show the same sheets that were hidden again.

so I did this, I set the sheets that needed to be hidden in very hidden mode in the properties window. now the workbook can be saved without having to run that script for the first time.

Also I canged something in the hide all function to make sure that the right sheet it visible:
for some reason the script would run through the sheets at random, and wouldn't unhide the warning sheet before hiding the rest, and you can't have all the sheets hidden. so I did this:

'this 'script hides the sheets except for the "warning" sheet
Sub HideAll()
Application.ScreenUpdating = False
ThisWorkbook.Worksheets(6).Visible = xlSheetVisible
For Each wsSheet In ThisWorkbook.Worksheets
If wsSheet.CodeName <> "Sheet7" Then
wsSheet.Visible = xlSheetVeryHidden
End If
Next wsSheet
Application.ScreenUpdating = True
End Sub

Now I just need to remember that when publishing the whole thing I need to set the sheets to their native very hidden status.

Thanks Rory, charlize and xld for helping me with this project!!!

I'm uploading a new version after I fiish polishing it.