Search code examples
excelvbaworksheet

Prompt User to Open a Workbook and to Select a Worksheet, opens an blank file


I would like the User to choose a Workbook and then select the worksheet that they need. The code works perfectly when it is Debug - Step Into. But, When the complete macro is run through the button, the file do get open and prompts to choose the sheet but no sheets or cells are visible. It's all BLANK. There is no protection to the file. Column names and Row numbers are not visible

 Sub LoadData()
     Dim ws As Worksheet
     Dim desiredSheetName As String
     Dim c As String

     Application.ScreenUpdating = False
     Application.DisplayAlerts = False

     ans = MsgBox("Choose the file to retrive the data?", vbYesNo, "Choose Source")
     If ans = vbYes Then
        myfile = Application.GetOpenFilename(, , "Browse for Workbook")
        If myfile <> False Then
           ThisWorkbook.Sheets("Destination").Range("AA2") = myfile
           Set src_data = Workbooks.Open(myfile)

          On Error Resume Next
          desiredSheetName = InputBox("Select any cell inside the target sheet: ",type:=8).worksheet.name 
          sht = desiredSheetName
          On Error GoTo 0

          Set dest = ThisWorkbook.Worksheets("Destination")

          src_data.Activate

          lastcell = src_data.Sheets(sht).Cells(Rows.Count, "C").End(xlUp).Row
          LastRowD = dest.Cells(dest.Rows.Count, "F").End(xlUp).Offset(0).Row

          src_data.Activate
          Sheets(sht).Select
          Range("A:B,D:D").Select
          Selection.Copy
          dest.Activate
          Range("F1").Select
          Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, skipblanks:=False, Transpose:=False

          src_data.Close False

          dest.Select

        End If
   Else
     Exit Sub
 End If

Application.DisplayAlerts = True
Application.ScreenUpdating = True
End Sub

Solution

  • You need to not turn screen updating off until after you ask for the range selection, because when the macro is running, the file will open but the screen doesn't update to show the cells.

    A few other pointers given your code:

    • You're using variables you haven't declared (sht, srcData).
    • If you're using a variable only once (like ans from your MessageBox), just plug that in directly instead of dimming a variable and using that. The exception would be when using a constant like a number. Always better in that case to use a meaningful variable name than a hardcoded number without context.
    • You should set Option Explicit at the top of your module and you'll be warned about this using undeclared variables.
    • You're getting destinationSheet and then setting sht to the same thing. Why not just get rid of sht altogether?
    • Rather than mixing variable naming conventions (src_data and desiredSheetName), just pick one and stick with it (I use the latter format myself).
    • Selecting and activating things is usually the wrong way to do it unless you're doing it so the user can see something specific. Usually you should just do the operations on the ranges and sheets themselves. Also, you should be explicit about the worksheet you're using (because otherwise it defaults to ActiveSheet). For example:

    Rather than:

    Range("A:B,D:D").Select
    Selection.Copy
    dest.Activate
    Range("F1").Select
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, skipblanks:=False, Transpose:=False
    

    Do:

    src_data.Range("A:B,D:D").Copy
    dest.Range("F1").PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, skipblanks:=False, Transpose:=False
    

    That makes it very clear where you're copying from and where you're pasting to, is fewer lines of code, and is also faster to process.

    So here's the final code, with clearly named, defined variables, no Selecting of sheets, non-used variables cut out.

    Option Explicit
    Sub LoadData()
         Dim sourcePath As String
         Dim sourceWorkbook As Workbook
         Dim sourceWorksheet As Worksheet
         Dim destinationWorksheet As Worksheet
         Dim lastSourceRow As Long
         Dim lastDestinationRow As Long
    
         'Application.ScreenUpdating = False '==>Moved after InputBox
         Application.DisplayAlerts = False
         Set destinationWorksheet = ThisWorkbook.Worksheets("Destination")
    
         If MsgBox("Choose the file to retrive the data?", vbYesNo, "Choose Source") = vbYes Then
            sourcePath = Application.GetOpenFilename(, , "Browse for Workbook")
            If sourcePath <> "False" Then
              destinationWorksheet.Range("A2") = sourcePath
              Set sourceWorkbook = Workbooks.Open(sourcePath)
    
              On Error Resume Next
              sourceWorksheet = Application.InputBox(prompt:="Select any cell inside the target sheet:", Type:=8).Worksheet
              On Error GoTo 0
    
              Application.ScreenUpdating = False
    
              lastSourceRow = sourceWorksheet.Cells(Rows.Count, "C").End(xlUp).Row
              lastDestinationRow = destinationWorksheet.Cells(destinationWorksheet.Rows.Count, "F").End(xlUp).Offset(0).Row
    
              sourceWorksheet.Range("A:B,D:D").Copy
              destinationWorksheet.Range("F1").PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, skipblanks:=False, Transpose:=False
    
              sourceWorkbook.Close False
    
              destinationWorksheet.Select
    
            End If
       Else
         Exit Sub
     End If
    
    Application.DisplayAlerts = True
    Application.ScreenUpdating = True
    
    End Sub