Font colour not being picked up

[Fred Newton]

Courtesy of this group I have set up the following Worksheet_Change
event, however i am experiencing a problem where by the fill colour
changes but the font does not, unless I use the tick button to accepr
the change to the cell. I I use the enter key, the font colour of the
cell below is changed, but not the fill colour.

Private Sub Worksheet_Change(ByVal Target As Range)

Dim vLetter As String
Dim vColor As Integer
Dim fcolour As Integer
Dim cRange As Range
Dim cell As Range

'***************** check range ****
Set cRange = Intersect(Range("A3:IV3"), Range(Target(1).Address))
If cRange Is Nothing Then Exit Sub
'**********************************

For Each cell In Target
vLetter = cell.Value & " "
vColor = 0 'default is no color

Select Case vLetter
Case "R ", "r "
vColor = 3
fcolour = 2
Case "A ", "a "
vColor = 27
fcolour = 1
Case "G ", "g "
vColor = 10
fcolour = 2
Case "C ", "c ", "D ", "d ", "P ", "p "
vColor = 41
fcolour = 2
Case "H ", "h "
vColor = 9
fcolour = 2
Case " "
vColor = 0
fcolour = 1
End Select

Application.EnableEvents = False 'should be part of Change macro
cell.Interior.ColorIndex = vColor
With Selection.Font
.ColorIndex = fcolour
End With
Application.EnableEvents = True 'should be part of Change macro

Next cell

End Sub

Can someone point out the error of my vb ways please ?

Thanks in advance
Fred Newton
Zurich Financial Services

 

[Frank Kabel]

Hi
try changing:
With Selection.Font
.ColorIndex = fcolour
End With

to
cell.font.colorindex=fcolour

 

[JE McGimpsey]

The answer to you question is you should replace

With Selection.Font
.ColorIndex = fcolour
End With

with
cell.Font.ColorIndex = fcolour

However, you can make several changes to make your code more efficient,
and easier to maintain:

Range(Target(1).Address) is equivalent to Target(1)

There's no need to set Application.EnableEvents to False - changing cell
or font color doesn't fire an event.

It doesn't appear to me that you need to add a space character - test
the cell's .Text property directly.

Also, if you test LCase(.Text), you only have to check for lowercase
characters

In your current code (corrected as above), if the value contains more
than one character, or if the character doesn't match one of the cases,
fcolour is (by default, since you didn't specify) 0. Technically, 0 is
an invalid value - setting the font colorindex to 0 actually sets it to
xlColorIndexAutomatic, or -4105:

Private Sub Worksheet_Change(ByVal Target As Range)
Dim vColor As Long
Dim fcolour As Long
Dim cRange As Range
Dim cell As Range
'***************** check range ****
Set cRange = Intersect(Range("A3:IV3"), Target(1))
If cRange Is Nothing Then Exit Sub
'**********************************
For Each cell In Target
With cell
Select Case LCase(.Text)
Case "r"
vColor = 3
fcolour = 2
Case "a"
vColor = 27
fcolour = 1
Case "g"
vColor = 10
fcolour = 2
Case "c", "d", "p"
vColor = 41
fcolour = 2
Case "h"
vColor = 9
fcolour = 2
Case ""
vColor = xlColorIndexNone
fcolour = 1
Case Else
vColor = xlColorIndexNone
fcolour = xlColorIndexAutomatic
End Select
.Interior.ColorIndex = vColor
.Font.ColorIndex = fcolour
End With
Next cell
End Sub

In the default color palette, font.colorindex = 1 sets the color to
black, as does font.colorindex = xlcolorindexautomatic. In that event,
you could eliminate the Case "" block and leave the Case Else block. If
you've changed color(1) in the color palette, you need to keep the Case
"" block.

Note that the way you currently use Target, if the top row of the
selection is Row 3, the entire selection is checked. So if you have
B3:J100 selected, a change in G17 will cause all the cells in B3:J100 to
be checked/formatted. Is that what you want?

If you only want cells in row 3 to be checked/formatted, change

For Each cell in Target

to

For Each cell In cRange

 

[Fred Newton]

Firstly, thanks to both Frank and JE for their responses, they both
solved my problem and have allowed me to pick up a little more
knowledge of VB along the way.

JE,

It doesn't appear to me that you need to add a space character - test
the cell's .Text property directly.

The reason I added a blank to the character was because row 3 contains
other cells that I didn't want affected by the macro. That said,
however, with a minor mod to your supplied code, namely the Case Else
block, i've set the remaining cells to the colour I need.

In the default color palette, font.colorindex = 1 sets the color to
black, as does font.colorindex = xlcolorindexautomatic. In that event,
you could eliminate the Case "" block and leave the Case Else block. If
you've changed color(1) in the color palette, you need to keep the Case
"" block.

Whilst I haven't changed color(1) I have retained the Case "" block
and set it to 3 to highlight to the user the fact that they haven't
completed the cell yet.

Note that the way you currently use Target, if the top row of the
selection is Row 3, the entire selection is checked. So if you have
B3:J100 selected, a change in G17 will cause all the cells in B3:J100 to
be checked/formatted. Is that what you want?

If you only want cells in row 3 to be checked/formatted, change

For Each cell in Target

to

For Each cell In cRange

Thanks for that one, it wasn't an option I had thought of, but in
testing the before and after effects you've shown a hole in the
coding.

Thanks for all your help with this.

Regards
Fred Newton

 

[Fred Newton]

One final query, i've seen your posting relating to forcing the cell
to change when drop-downs are used (by use of the Worksheet Calculate
event), how can I wrap this into the code i've now got. I tried
(admittedly slightly ham-fistedly) to bed the code in place of the
worksheet name change you had originally supplied, but ended up with
an error "Sub or Function not defined" and the Target keyword
highlighted on the Set cRange statement.

Set cRange = Intersect(Range("A3:IV3"), (Target(1)))

Thanks for any help you can give
Regards
Fred Newton