Solved: VBA formatting

[bdavids3]

I wrote the following code to format my worksheet based on certain conditions. I'm new to VB, so I'm not even sure if this is the correct approach. What I'm trying to do is color certain rows from column A through J based primarily on the contents of column C. There is only 1 exception to this, which also checks to see if column D starts with a certain string.

Right now I get a run-time error '13' on the last if statement (10 lines from the bottom under Case "Task".

I also noticed that if I paste a large amount of data (~1000 rows) it takes forever to update and actually usually freezes Excel to the point where I have to force-quit the application. I know my data set will always be less than 1000 rows, but I can't be sure whether it's 800 one time and 900 the next. The actual data set I want to format is A3:Jyyy where yyy is no larger than 1000.

Here is my code so far:


Option Compare Text
Option Explicit
Private Sub Worksheet_Change(ByVal Target As Range)

Dim Cell As Range
Dim Rng1 As Range

On Error Resume Next
Set Rng1 = ActiveSheet.Cells.SpecialCells(xlCellTypeFormulas, 1)
On Error GoTo 0
If Rng1 Is Nothing Then
Set Rng1 = Range(Target.Address)
Else
Set Rng1 = Union(Range(Target.Address), Rng1)
End If
For Each Cell In Rng1
Select Case Cell.Value
Case "Phase"
Cell.Interior.ColorIndex = 53 'tomato red
Cell.Font.Bold = True
Cell.Font.ColorIndex = 2
Case "Thread"
Cell.Interior.ColorIndex = 1 'black
Cell.Font.Bold = True
Cell.Font.ColorIndex = 2
Case "Activity"
Cell.Interior.ColorIndex = 15 'light grey
Cell.Font.Bold = True
Case "Task"
If Cells(Cell, 4) = Left("Approve", 7) Then
Cell.Interior.ColorIndex = 40 'light orange
Cell.Font.Italic = True
End If
Case Else
'do nothing to leave whatever formatting is already in place
End Select
Next
End Sub

Thanks,

Brian

[Oorang]

:welcome:
Hi Brian,
Welcome to the board! I see a couple potential problems here. First and formost is that the "Cell" object is a range. The Range object's default property is "Value" so when you use "Cell" with nothing after it, it is the same as using Cell.Value. Meanwhile you are using it within the Cells() call as a row argument. So it is pulling the value of the current cell and placing that as the current row argument (which if it is blank or has a text value you will get an error). I think maybe you meant to do Cell.Row ?

The other problem I see, will not cause an error, just slowdown. Left("Approve", 7) is the same thing as "Approve". Why use the Left Function at all?

Does that clean it up for you?

[bdavids3]

:welcome:
Hi Brian,
Welcome to the board! I see a couple potential problems here. First and formost is that the "Cell" object is a range. The Range object's default property is "Value" so when you use "Cell" with nothing after it, it is the same as using Cell.Value. Meanwhile you are using it within the Cells() call as a row argument. So it is pulling the value of the current cell and placing that as the current row argument (which if it is blank or has a text value you will get an error). I think maybe you meant to do Cell.Row ?

The other problem I see, will not cause an error, just slowdown. Left("Approve", 7) is the same thing as "Approve". Why use the Left Function at all?

Does that clean it up for you?

During the course of the day I was looking at my code a little closer and realized there is no where that I specify that I want the select statement to only check the value in column C. Also, do you think I would need a loop in each of the case statements so that it only formats cells A through J in a row? If so, what is the best way to accomplish this?

The Left("Approve", 7) checks to see if the cell starts with "Approve" since the contents of column D is some kind of sentence and I'm only looking for the ones that start with that string.

Thanks,

Brian

[p45cal]

.. specify that I want the select statement to only check the value in column C.
set rng1 = ActiveSheet.columns("C").SpecialCells(xlCellTypeFormulas, 1)perhaps?


..Also, do you think I would need a loop in each of the case statements so that it only formats cells A through J in a row? If so, what is the best way to accomplish this?That depends on whether yuo want to format all cells from A to J in that row or just those cells that fulfil the criteria.


.. The Left("Approve", 7) checks to see if the cell starts with "Approve" since the contents of column D is some kind of sentence and I'm only looking for the ones that start with that string. No it doesn't! I feel you may want
If left(Cells(Cell.row, 4),7) = "Approve" Theninstead of
If Cells(Cell, 4) = Left("Approve", 7) Then

[Oorang]

During the course of the day I was looking at my code a little closer and realized there is no where that I specify that I want the select statement to only check the value in column C. Use the Excel.Intersect method to limit your range to column C (see example below).

Also, do you think I would need a loop in each of the case statements so that it only formats cells A through J in a row? If so, what is the best way to accomplish this?I'd just specify the range (see example below).

The Left("Approve", 7) checks to see if the cell starts with "Approve" since the contents of column D is some kind of sentence and I'm only looking for the ones that start with that string.Right, but you want to apply the left method to cell value, not the word "Approve", (see example below).Private Sub Worksheet_Change(ByVal Target As Range)

Dim Cell As Range
Dim Rng1 As Range

On Error Resume Next
Set Rng1 = ActiveSheet.Cells.SpecialCells(xlCellTypeFormulas, 1)
On Error GoTo 0

If Rng1 Is Nothing Then
Set Rng1 = Range(Target.Address)
Else
Set Rng1 = Union(Range(Target.Address), Rng1)
End If
Set Rng1 = Excel.Intersect(Rng1, Me.Columns(3), Me.UsedRange)
If Not Rng1 Is Nothing Then
For Each Cell In Rng1
With Me.Range(Me.Cells(Cell.Row, 1), Me.Cells(Cell.Row, 10))
Select Case Cell.Value
Case "Phase"
.Interior.ColorIndex = 53 'tomato red
.Font.Bold = True
.Font.ColorIndex = 2
Case "Thread"
.Interior.ColorIndex = 1 'black
.Font.Bold = True
.Font.ColorIndex = 2
Case "Activity"
.Interior.ColorIndex = 15 'light grey
.Font.Bold = True
Case "Task"
If Left$(Cells(Cell.Row, 4).Value, 7) = "Approve" Then
.Interior.ColorIndex = 40 'light orange
.Font.Italic = True
End If
Case Else
'do nothing to leave whatever formatting is already in place
End Select
End With
Next
End If
End Sub

[bdavids3]

Use the Excel.Intersect method to limit your range to column C (see example below).
I'd just specify the range (see example below).
Right, but you want to apply the left method to cell value, not the word "Approve", (see example below

That's it exactly! Thanks for all your help.

Cheers,

Brian