Code seems to be searching backwards.

XrayLemi

Board Regular
Joined
Aug 1, 2018
Messages
87
Office Version
  1. 365
Platform
  1. Windows
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.

VBA Code:
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.
VBA Code:
 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
 

Some videos you may like

Excel Facts

Ambidextrous Undo
Undo last command with Ctrl+Z or Alt+Backspace. If you use the Undo icon in the QAT, open the drop-down arrow to undo up to 100 steps.

JLGWhiz

Well-known Member
Joined
Feb 7, 2012
Messages
12,979
Office Version
  1. 2013
Platform
  1. Windows
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'.

VBA Code:
 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

Well-known Member
Joined
Dec 23, 2017
Messages
1,738
Office Version
  1. 2010
Platform
  1. Windows
I have spotted another error which is probably not related but you have a line:
VBA Code:
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

Well-known Member
Joined
Feb 7, 2012
Messages
12,979
Office Version
  1. 2013
Platform
  1. Windows
I have spotted another error which is probably not related but you have a line:
VBA Code:
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

Board Regular
Joined
Aug 1, 2018
Messages
87
Office Version
  1. 365
Platform
  1. Windows

ADVERTISEMENT

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...
VBA Code:
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

Board Regular
Joined
Aug 1, 2018
Messages
87
Office Version
  1. 365
Platform
  1. Windows
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

Well-known Member
Joined
Feb 7, 2012
Messages
12,979
Office Version
  1. 2013
Platform
  1. Windows

ADVERTISEMENT

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

Well-known Member
Joined
Feb 7, 2012
Messages
12,979
Office Version
  1. 2013
Platform
  1. Windows
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.
VBA 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

Board Regular
Joined
Aug 1, 2018
Messages
87
Office Version
  1. 365
Platform
  1. Windows
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

Well-known Member
Joined
Feb 7, 2012
Messages
12,979
Office Version
  1. 2013
Platform
  1. Windows
With all that being said, is the fix I made to the code what you wanted me to do?


If you changed this

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

to this

VBA Code:
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.
 
Solution

Watch MrExcel Video

Forum statistics

Threads
1,127,785
Messages
5,626,869
Members
416,208
Latest member
tan21

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Top