0

I have a file that is exported by the system each week that needs to be modified slightly in each sheet and all sheets to be renamed based on one cell in that particular cell (E7). I am not able to get it to loop no matter how hard i try. Any ideas what i am missing? I assume it has to do with that 'konstandst' variable and how i name sheets, but can fix..

Sub Formateraom()
    ' Format and change name of the sheet
    Dim ws As Worksheet
    Dim weekNR As Variant
    Dim konstnadst As Variant

    weekNR = InputBox("What week number is it?")
    For Each ws In Worksheets
        Set ws = ActiveSheet
        konstnadst = Range("E7")
        Range("A2:C2").Select
        Selection.ClearContents
        Range("A5:T5").Select
        Selection.ClearContents
        Columns("C:C").ColumnWidth = 75#
        Rows("5:7").Select
        With Selection
            .VerticalAlignment = xlBottom
            .WrapText = True
            .Orientation = 0
            .AddIndent = False
            .IndentLevel = 0
            .ShrinkToFit = False
            .ReadingOrder = xlContext
        End With
        With Selection
            .VerticalAlignment = xlCenter
            .WrapText = True
            .Orientation = 0
            .AddIndent = False
            .IndentLevel = 0
            .ShrinkToFit = False
            .ReadingOrder = xlContext
        End With
        With Selection
            .HorizontalAlignment = xlCenter
            .VerticalAlignment = xlCenter
            .WrapText = True
            .Orientation = 0
            .AddIndent = False
            .IndentLevel = 0
            .ShrinkToFit = False
            .ReadingOrder = xlContext
        End With
        Columns("H:H").ColumnWidth = 13
        Range("H7,M7,G7").Select
        Range("G7").Activate
        Selection.NumberFormat = "m/d/yyyy"
        Columns("M:M").ColumnWidth = 13
        Columns("G:G").ColumnWidth = 13
        Range("C3").Select
        ActiveCell.FormulaR1C1 = weekNR
        Range("C4").Select
        With Selection
            .HorizontalAlignment = xlRight
            .VerticalAlignment = xlCenter
            .WrapText = False
            .Orientation = 0
            .AddIndent = False
            .IndentLevel = 0
            .ShrinkToFit = False
            .ReadingOrder = xlContext
            .MergeCells = False
        End With
        Range("C3").Select
        With Selection
            .HorizontalAlignment = xlRight
            .VerticalAlignment = xlCenter
            .WrapText = False
            .Orientation = 0
            .AddIndent = False
            .IndentLevel = 0
            .ShrinkToFit = False
            .ReadingOrder = xlContext
            .MergeCells = False
        End With
        With Selection
            .HorizontalAlignment = xlCenter
            .VerticalAlignment = xlCenter
            .WrapText = False
            .Orientation = 0
            .AddIndent = False
            .IndentLevel = 0
            .ShrinkToFit = False
            .ReadingOrder = xlContext
            .MergeCells = False
        End With
        ActiveSheet.Name = "Fakturaunderlag " & konstnadst & " " & weekNR
    Next
End Sub

Sending gigantic ball of karma to whoever can point me to the right direction!

Cindy Meister
  • 25,071
  • 21
  • 34
  • 43
Irina Ban
  • 5
  • 3

2 Answers2

0

Set ws = ActiveSheet would always set the current loop sheet (i.e. ws) to the currently Active one.

this way you'd always get the same sheet, the one being active before the loop begins

so you'd simply have to change

Set ws = ActiveSheet

to

ws.Activate

thus making the current loop sheet the active one


but although tha above patch may (seem to) work, it is also a bad coding habit and you're warmly invited to avoid Activate/ActiveXXX/Select/Selection pattern and switch to a direct and qualified up to the worksheet (and workbook, if there could be more than one open at the time the macro is being run) Range reference

so your code could become the following:

Option Explicit

Sub Formateraom()
    ' Format and change name of the sheet
    Dim ws As Worksheet
    Dim weekNR As Variant
    Dim konstnadst As Variant

    weekNR = InputBox("What week number is it?")
    For Each ws In Worksheets
        With ws ' reference the current loop sheet. inside the 'With ... End With' block, all its members are accessed by means of a dot (.)
            konstnadst = .Range("E7") ' initialize 'konstnadst' to referenced sheet cell E7 value
            .Range("A2:C2").ClearContents
            .Range("A5:T5").ClearContents
            .Columns("C:C").ColumnWidth = 75#
            With .Rows("5:7") ' reference referenced sheet rows 5 to 7
                .VerticalAlignment = xlBottom
                .WrapText = True
                .Orientation = 0
                .AddIndent = False
                .IndentLevel = 0
                .ShrinkToFit = False
                .ReadingOrder = xlContext

                .VerticalAlignment = xlCenter
                .WrapText = True
                .Orientation = 0
                .AddIndent = False
                .IndentLevel = 0
                .ShrinkToFit = False
                .ReadingOrder = xlContext

                .HorizontalAlignment = xlCenter
                .VerticalAlignment = xlCenter
                .WrapText = True
                .Orientation = 0
                .AddIndent = False
                .IndentLevel = 0
                .ShrinkToFit = False
                .ReadingOrder = xlContext
            End With

            .Columns("H:H").ColumnWidth = 13
            .Range("H7,M7,G7").NumberFormat = "m/d/yyyy"
            .Columns("M:M").ColumnWidth = 13
            .Columns("G:G").ColumnWidth = 13
            .Range("C3").FormulaR1C1 = weekNR

            With .Range("C4") ' reference referenced sheet cell C4
                .HorizontalAlignment = xlRight
                .VerticalAlignment = xlCenter
                .WrapText = False
                .Orientation = 0
                .AddIndent = False
                .IndentLevel = 0
                .ShrinkToFit = False
                .ReadingOrder = xlContext
                .MergeCells = False
            End With

            With .Range("C3") ' reference referenced sheet cell C3
                .HorizontalAlignment = xlRight
                .VerticalAlignment = xlCenter
                .WrapText = False
                .Orientation = 0
                .AddIndent = False
                .IndentLevel = 0
                .ShrinkToFit = False
                .ReadingOrder = xlContext
                .MergeCells = False

                .HorizontalAlignment = xlCenter
                .VerticalAlignment = xlCenter
                .WrapText = False
                .Orientation = 0
                .AddIndent = False
                .IndentLevel = 0
                .ShrinkToFit = False
                .ReadingOrder = xlContext
                .MergeCells = False
            End With

            .Name = "Fakturaunderlag " & konstnadst & " " & weekNR ' change the name of the referenced sheet
        End With
    Next
End Sub
DisplayName
  • 13,283
  • 2
  • 11
  • 19
  • Holy moly - this works like magic! What i realized is that the problem was not at all where i thought i was. I recorded the macro for the stuff that needed to be formatted (as i am not good at macro and i was trying to save time on googling) and that is where I screwed up. Never again will i listen to smbd saying "you can just record macro!". Thank you veeery much for your help!!! – Irina Ban Aug 31 '18 at 15:26
  • You are welcome. Recording a macro can be a good start to see what objects are being used and which methods/properties of them are leveraged for a given purpose. But you have to take some VBA class (there are plenty of them in the web) to effectively code. – DisplayName Aug 31 '18 at 15:33
  • @DisplayName You don't *need* to take classes to learn how to program in VBA; you could just google tutorials, ask questions on here, record macros to glean information from excel. Why pay the money for a VBA class when there are *plenty* of online resources to learn at your own pace for free? – jcrizk Aug 31 '18 at 15:38
  • now that i have this bit working how do i connect the script to export excels? code works separately, but not when i put it after the above code: `Sub SparaSeparataFiler() Dim ws As Worksheet Dim xPath As String xPath = Application.ActiveWorkbook.Path Application.ScreenUpdating = False Application.DisplayAlerts = False For Each ws In ThisWorkbook.Sheets nm = ws.Name ws.Copy Application.ActiveWorkbook.SaveAs Filename:=xPath & "\" & nm & ".xlsx" Application.ActiveWorkbook.Close False Next Application.DisplayAlerts = True Application.ScreenUpdating = True End Sub` – Irina Ban Aug 31 '18 at 15:39
  • what i am trying to do is to prompt from a "macro file" to open the file that i want to do the manipulations to. The first sub goes well, but after that one it seems to be jumping back to the "macro file". I tried t´calling sub after sub, that did not help either.. – Irina Ban Aug 31 '18 at 15:44
  • if you are stuck with a new issue, post a new answer along with the code and the issue description – DisplayName Aug 31 '18 at 17:38
  • @jcrizk, I was actually thinking of free resources. English is not my mother language so I didn't know that a _class_ would imply payment – DisplayName Aug 31 '18 at 17:40
  • @DisplayName gotcha. Here it *usually* implies payment, but on rare occasions it doesn't. – jcrizk Aug 31 '18 at 18:07
0

The below question is actually a case where you need to use .Select. In almost every other instance, never use it.

Question regarding how to export xlsx as pdf

This question should help you learn how to open an excel file using a file dialog box

I'm unsure of what you want to do with weekNR considering you're doing this with it later:

ActiveCell.FormulaR1C1 = weekNR

So I'm going to ignore it until I get more info about it.

Since konstnadst is a Range object, as you have assigned it, I would suggest declaring it as a Range object, with a reference to the worksheet you're working with, like so:

Dim konstandadst As Range
'you need to Set objects such as Ranges, Worksheets, Workbooks, ect.
Set konstandadst = whateverWsThisIs.Range("E7")

Using Range.Activate is the equivalent of clicking the range, which seems to be useless in your circumstance, so get rid of that.

Using:

Range1.Select
Range2.Select
Range3.Select

Results in you only selecting Range3 when this block finishes.

I would highly suggest never using .Select, and instead create reference variables to your ranges to work with them directly like so:

'these select cell A1
Set MyRange = ws.Range("A1")
Set MyRange = ws.Cells(1,1)

'this selects column B
Set MyRange = ws.Range("B:B")

'this selects the row from A1 to B1
Set MyRange = ws.Range(ws.Cells(1,1), ws.Cells(1,2))

'this selects a table defined from A1 to C2
Set MyRange = ws.Range(ws.Cells(1,1), ws.Cells(2,3))

Don't do this:

For Each ws In Worksheets

Do this because you want to explicitly tell VBA the workbook in which you're referencing the Worksheets collection:

For Each ws In ThisWorkbook.Worksheets

Or if you're a freak like me:

For Each ws In Excel.Application.ThisWorkbook.Worksheets

Here are a few relevant operations you can do with Range objects (more here):

'clears the values in the cells
MyRange.ClearContents
'clears the formatting and formulas in the cells
MyRange.Clear
'adjust column width
MyRange.ColumnWidth = someNumber
'adjust row height
MyRange.RowHeight = someOtherNumber
'eliminate indents (i think)
MyRange.IndentLevel = 0
'change the orientation
MyRange.Orientation = 0

After you have set reference variables to the ranges you want, you can use them like this:

With MyRange
    'do the stuff here
End With

Instead of:

With Selection
    'bad stuff here, don't do the stuff
End With
jcrizk
  • 605
  • 5
  • 15
  • I realise now that the problem is the fact that i was literally looping through the same cells and that is all because i recorded the macro. I will now be able to sort this out thanx to your help! Thank you! – Irina Ban Aug 31 '18 at 15:31