0

The below code opens up folders and allows me to choose what document I want to be the source, it then opens it behind screen and works when copying sheets.

I tried to change the code to copy rows based on column G having "140. On Hold" in it, then pasting each of these rows into the active workbook.

UPDATED CODE

Sub GetBIDFileCopyData()

Dim Fname As String
Dim SrcWbk As Workbook
Dim DestWbk As Workbook
Dim C As Range
Dim J As Long

Set DestWbk = ThisWorkbook

Fname = Application.GetOpenFilename(FileFilter:="Excel Files (*.xls*), *.xls*", Title:="Select a File")
If Fname = "False" Then Exit Sub
Set SrcWbk = Workbooks.Open(Fname)

SrcWbk.Sheets("ChangeDetails").Rows(C.Row).Copy DestWbk.Sheets("Bids On-Hold 29.01.20").Rows(J)
J = 1
For Each C In SrcWbk.Range("G2:G200")
    If C.Value = "140. On Hold" Then
        J = J + 1
    End If
Next C

SrcWbk.Close False

End Sub
Community
  • 1
  • 1
Hsmith
  • 7
  • 4
  • What are `SrcWbk.Sheets ("ChangeDetails")` and `DestWbk.Sheets ("Bid Delivery Report")` supposed to do? – BigBen Feb 13 '20 at 13:34
  • Two issues i see are `Dim J As Integar`. First the spelling is incorrect and also its best to use `As Long`, So i would refactor using `Dim J As Long` also instead of `If C = "140. On Hold" Then` use `If C.Value = "140. On Hold" Then`. – Zack E Feb 13 '20 at 13:41
  • compile error is returned when I try to run it? fyi i'm using Excel 2010 – Hsmith Feb 13 '20 at 13:44
  • Use Autofilter as shown [HERE](https://stackoverflow.com/questions/11631363/how-to-copy-a-line-in-excel-using-a-specific-word-and-pasting-to-another-excel-s/11633207#11633207) – Siddharth Rout Feb 13 '20 at 13:58
  • that's all a bit confusing, however I need to keep the option in where I choose the source file, as the name of it changes every week due to the dates on it changing. Help is much appreciated – Hsmith Feb 13 '20 at 14:03

2 Answers2

0

The two lines

SrcWbk.Sheets ("ChangeDetails")
DestWbk.Sheets ("Bid Delivery Report")

do not compile. What are they supposed to do?

You try to copy the rows with the following code:

SrcWbk.Rows(C.Row).Copy DestWbk.Rows(J)

but you are missing a reference to the worksheet.

So maybe you are looking for:

    SrcWbk.Sheets("ChangeDetails").Rows(C.Row).Copy DestWbk.Sheets("Bid Delivery Report").Rows(J)

or better use varables for your sheets.

Jochen
  • 1,254
  • 1
  • 7
  • 9
  • Still not working, the above code works better, however I get Application-defined or Object-defined error returned?? I am new to VBA to very unsure, so bare with me. – Hsmith Feb 13 '20 at 13:53
0

As @SiddharthRout commented, the best way to copy/paste based on a specific criteria, is to use a filter. Comments are given in the code below. I did not test your code to open a file.

Dim Fname As String, SrcWbk As Workbook, DestWS As Worksheet, Rng As Range 'Assign your variables

    'Set your destination worksheet
    Set DestWS = ThisWorkbook.Sheets("Bids On-Hold 29.01.20")

    Fname = Application.GetOpenFilename(FileFilter:="Excel Files (*.xls*), *.xls*", Title:="Select a File")

        If Fname = "False" Then Exit Sub

    Set SrcWbk = Workbooks.Open(Fname)

    'Set the range you want to filter on your scorce worksheet
    Set Rng = SrcWbk.Sheets("ChangeDetails").Range("G2:G200")

    'Since you used only column G for your range, use the copy line below.
    'But if you use the full range of the worksheet, e.g. Range("A1:Z200"),
    'you could use field:=7 in the filter, and remove .EntireRow from the copy line

        With Rng
            'Filter Column G
            .AutoFilter field:=1, Criteria1:="140. On Hold"
            'use Resize and Offset to copy the visible data
            'If Row 2 has data and is not a header row, you should use Row 1, in Rng
            'Offset and Resize adjusts the range so the top row(Header) is not copied
            Rng.Resize(Rng.Rows.Count - 1).Offset(1).SpecialCells(xlCellTypeVisible).EntireRow.Copy
            DestWS.Range("A1").PasteSpecial xlPasteValues

        'Clear the filter    
        .AutoFilter
        End With
GMalc
  • 2,608
  • 1
  • 9
  • 16