Code seems to be searching backwards.

[XrayLemi]

Hello again all. Hope you are safe and healthy.
I have the following code on multiple sheets in a workbook. I cannot use XL2BB because of security reasons. so I will have to post it the hard way.

Private Sub CommandButton1_Click()
    UpdateDataFromMasterFile
End Sub
Private Sub Worksheet_Change(ByVal Target As Range)
  Dim r As Range, c As Range
  Set r = Union(Range("J6:J5000"), Range("G6:G5000"))
  Set r = Intersect(Target, r)
  If Not r Is Nothing Then
   Application.EnableEvents = False
   For Each c In r
    Select Case True
       Case 10 = c.Column 'J
        If c.Value = "" Then
          Cells(c.Row, "L").Value = ""
          Cells(c.Row, "L").Locked = True
          Else
          Cells(c.Row, "L").Locked = False
        End If
       Case 7 = c.Column 'G
        If c.Value = "Not Listed" Then
          Cells(c.Row, "H").Locked = False
          Else
          Cells(c.Row, "H").Locked = True
          Cells(c.Row, "H").Value = ""
        End If
       Case Else
    End Select
   Next c
  End If
  
If Target.Cells.Count > 3 Then Exit Sub
  If Not Intersect(Target, Range("C6:C5000")) Is Nothing Then
   With Target(1, 3)
    .Value = Date
    .EntireColumn.AutoFit
   End With
  End If
 
    Dim p As Range, z As Range
     Set p = Range("M6:M5000")
     Set p = Intersect(Target, p)
     If Not p Is Nothing Then
   Application.EnableEvents = False
     For Each z In p
      Select Case True
       Case 13 = z.Column 'M
        If z.Value <> "" Then
         Check = MsgBox("Are your entries correct?" & vbCrLf & "After entering yes, These values CANNOT be changed.", vbYesNo + vbQuestion, "Cell Lock Notification")
           If Check = vbYes Then
            Target.Rows.EntireRow.Locked = True
            Cells(z.Row + 1, "B").Locked = False
            Cells(z.Row + 1, "C").Locked = False
            Cells(z.Row + 1, "D").Locked = False
            Cells(z.Row + 1, "F").Locked = False
            Cells(z.Row + 1, "G").Locked = False
            Cells(z.Row + 1, "I").Locked = False
            Cells(z.Row + 1, "J").Locked = False
            Cells(z.Row + 1, "K").Locked = False
            Cells(z.Row + 1, "M").Locked = False
            If Cells(Application.ActiveCell.Row, 17).Value <> "" Then Copyemail 'Q
            If Cells(Application.ActiveCell.Row, 18).Value <> "" Then ThisWorkbook.Save 'R
            With Me
                .Parent.Activate
                .Activate
                .Range("B" & Rows.Count).End(xlUp).Offset(1).Activate
            End With
           Else
            Cells(z.Row, "M").Value = ""
           End If
          End If
      Case Else
    End Select
   Next z
 End If
      Application.EnableEvents = True
    
End Sub

This line of code is giving me a headache.

 If Cells(Application.ActiveCell.Row, 17).Value <> "" Then Copyemail 'Q

If the active row column 17 cell is blank, nothing should happen. If the active row column 17 cell has a value, the module Copyemail should run. BUT, it should only run for the active row you are working on and only if the column 17 cell has an entry.

What I am finding is there are times when the code will at random, go BACK to find the last row that has a column 17 entry, and run the module. This happens even if the column 17 cell in the active row is blank. It also happens on random computers. This should not be happening.

I know this is long winded but I am trying to be thorough.

Thank you in advance,
Jim

 

[JLGWhiz]

Using ActiveCell in a For Each loop is not advisable, since the ActiveCell could be anywhere on the sheet but where you think it is. If you want to reference the cell being addressed on the same iteration of a loop, then use your range variabled that the loop initializes, in this case 'z'.

 If Cells(z.Row, 17).Value <> "" Then Copyemail 'Q
If Cells(z.Row, 18).Value <> "" Then ThisWorkbook.Save 'R

P.S. You do not need XL2BB to post code. You did fine with that.

 

[offthelip]

I have spotted another error which is probably not related but you have a line:

If Target.Cells.Count > 3 Then Exit Sub

If you hit this line then you exit the sub with
Application.EnableEvents = False
So nothing will work after that. Can I suggest you enable events before exiting the sub

 

[JLGWhiz]

I have spotted another error which is probably not related but you have a line:

If Target.Cells.Count > 3 Then Exit Sub

If you hit this line then you exit the sub with
Application.EnableEvents = False
So nothing will work after that. Can I suggest you enable events before exiting the sub

the code looks to me like it was once more than one macro that has been combined into one Worksheet_Change event procedure and that line of code should actually be at the top of the procedure to limit the number of cells that can be addressed for the procedure to act on. I would just move the line of code to right above the Dim statements.

 

[XrayLemi]

Okay folks,
I am by far not fluent in VBA programming. I got as far as I did with a ton of help. Mostly from this forum.

Offthelip, I did that part with allot of copy / paste / manipulate. This probably has not been a problem because the target cell has to be filled out before you can go any farther. It is never empty. I may have to figure a better way of auto adding a date if this could be a possible problem.

JLGWhiz, I Think this is what you meant...

If Cells(z.Row, "Q").Value <> "" Then Copyemail 'Q
If Cells(z.Row, "R").Value <> "" Then ThisWorkbook.Save 'R

I have tried it and it seems to work as it should. Although the previous code also seemed to work at first as well.

 

[XrayLemi]

GLGWhiz, I can only assume (i know bad thing to do) that the way I re-did the code now makes it specific to the row I am working on. The other way the code can "search" for any row "it" feels makes sense.
Am I on the right track?

 

[JLGWhiz]

GLGWhiz, I can only assume (i know bad thing to do) that the way I re-did the code now makes it specific to the row I am working on. The other way the code can "search" for any row "it" feels makes sense.
Am I on the right track?

After you create a few macros, you well come to learn that the active cell on a sheet is not necessarily the cell that the code is telling Execel to do something with or to. It is possible with vba that Cell A1 can be the active cell throughout an entire procedure that searches columns and rows for criteria and then copies or calculates or some other actions. And cell A1 never loses its active cell attributes. This in true in code that does not use the Select and Activate methods to send instructions to Excel. The direct method of coding does not require a sheet and then the range to be activated or selected in order to cause and action to occur on them, therefor the active cell does not come into play. So when you have a For...Next loop, or a For i = loop that walks through a certain range of cells, you have to use the variable in the For statement as a cell marker. There is a lot more to know about this than I can explain in a reply box. I suggest you search the web on 'using For statements in vba' and see if you can find a tutorial or an article that gives more details. Or you could check out a book store for a book that tells you how to do programming in vba. Maybe even find one on this website in the MrExcel store.

 

[JLGWhiz]

Here is a snippet that might help to understand what I am talking about. Copy this to code module1 in a new workbook. Then with a blank active sheet, run the code and read the message boxes that pop up. You have both a For i and a For Each loop in the code.

Sub tDemo()
Dim i As Long, c As Range
Range("A1").Activate
    For i = 1 To 5
        Cells(i, "D") = i
        MsgBox "Active Cell Is " & ActiveCell.Address
    Next
    For Each c In Range("D1:D5")
        MsgBox "Active Cell Is " & ActiveCell.Address & vbLf & "Target Cell Is " & c.Address
    Next
End Sub

 

[XrayLemi]

I fully understand I have ALLOT to learn about this. I also know you cannot explain it to me in one reply. I do have several books on VBA (one from here). I have Been thrown into this with no previous programming experience. I am learning on the fly. Cut / paste / massage / re-do / hope. It is sometimes hard for me to figure things out from a book when the answer I need is in chapter ten, but I haven't read chapters one through nine to learn how to get there.

I ran the module you created for me. I better see what you mean about active cell attributes. It also showed me that just because a cell is selected, it does not make that cell the active cell either.

With all that being said, is the fix I made to the code what you wanted me to do?
Thanks,
Jim

 

[JLGWhiz]

With all that being said, is the fix I made to the code what you wanted me to do?

If you changed this

If Cells(Application.ActiveCell.Row, 17).Value <> "" Then Copyemail 'Q
If Cells(Application.ActiveCell.Row, 18).Value <> "" Then ThisWorkbook.Save 'R

to this

If Cells(z.Row, "Q").Value <> "" Then Copyemail 'Q
If Cells(z.Row, "R").Value <> "" Then ThisWorkbook.Save 'R

Then that is what I waw suggesting.