Help with an Excel VBA Script (Excel 2010)

[nt1701]

Hello VBA Experts,

I am trying to debug an Excel VBA script that I wrote, and I am stuck. I have a drop-down menu in cell E11 and based on the option selected I wish to change the interior color (to gray) and lock-out cell range G21:H24. The worksheet is password protected and so I account for this in my VBA script. I am getting a variety of run-time errors each time I test the worksheet. Any ideas as to what is wrong with my code? Thanks.

I am using Excel 2010 and the code below uses the Worksheet object and the Change procedure:

Private Sub Worksheet_Change(ByVal Target As Range)
If Left(ActiveSheet.Cells(11, 5).Text, 7) = "Track D" Then
ActiveSheet.Unprotect "opt123"
ActiveSheet.Range(Cells(21, 7), Cells(24, 8)).Locked = False
ActiveSheet.Range(Cells(21, 7), Cells(24, 8)).Interior.ColorIndex = 16
ActiveSheet.Protect "opt123", True, True, False, True, True
Else
ActiveSheet.Unprotect "opt123"
ActiveSheet.Range(Cells(21, 7), Cells(24, 8)).Locked = False
ActiveSheet.Range(Cells(21, 7), Cells(24, 8)).ClearContents
ActiveSheet.Range(Cells(21, 7), Cells(24, 8)).Interior.ColorIndex = 16
ActiveSheet.Range(Cells(21, 7), Cells(24, 8)).Locked = True
ActiveSheet.Protect "opt123", True, True, False, True, True
End If
End Sub

[p45cal]

• 1. This event handler detects changes in the sheet, but it also changes the sheet, so the handler ends up being called repeatedly by itself. This is stopped by the Application.EnableEvents=True/False lines.
• 2. Unqualified references to ranges and other things in code in a sheet's code-module refer to the sheet itself, so many ActiveSheet instances are redundant. I've removed them.
• 3. I've assumed that cell E11 is unlocked.
• 4. You've used True as the 5th argument in the Protect statement meaning that the UserInterfaceOnly argument is True. This means that you don't need to protect/unprotect the sheet repeatedly (UserInterfacceOnly being True allows code to alter the sheet, but not users). The Protect line is still in there but it doesn't need to be there at all if the equivalent line is elsewhere, say in the Workbook_Open() event in the ThisWorkbook code-module, with the likes of:

  Private Sub Workbook_Open()
  Sheets("Sheet1").Protect "opt123", True, True, False, True, True
  End Sub

  adjusted, of course if that sheet's name isn't Sheet1.
• 5. Several lines of your code are the same on both sides of the If..Then..Else, meaning they'll be executed whether the condition is true or not, so they don't need to be within the If..Then..Else at all. I've taken them outside that construct.
• 6. I've used A1 style references instead of cells(row,column) references because it's easier for me to understand. There's nothing wrong with using your type references.
• 7. To avoid repeatedly referring to the same range I've put that range in a With..End With clause.
• 8. I've introduced a new boolean variable IsntTrackD which I'll leave you to fathom!

Private Sub Worksheet_Change(ByVal Target As Range)
Application.EnableEvents = False
Protect "opt123", True, True, False, True, True
IsntTrackD = Left(Range("E11").Text, 7) <> "Track D"
With Range("G21:H24")
  .Locked = IsntTrackD
  .Interior.ColorIndex = 16
  If IsntTrackD Then .ClearContents
End With
Application.EnableEvents = True
End Sub

ps. Do you really need to .Interior.ColorIndex = 16 each time? (Yes if the user's likely to change those cells' colouring.)

[nt1701]

Hello p45cal,

Thank you so much for making the time to review my code and for providing a remedy. I will go ahead and implement your remedy, test the worksheet, and provide feedback...hopefully with good news.

[nt1701]

Hello p45cal,

I did implement the VBA code version that you suggested and it worked!!! I made a couple of minor adjustments - following your code logic - and the worksheet is performing as expected. Thank you very much for your help - it is truly appreciated. Cheers!