And cleaned up the admin UI while talking to timplunkett.

1) We made the node type switch dynamic (there are not many types)
2) taxonomy keeps static (needs reload) else we have to load every single taxonomy term
3) splitted og, node types and taxonmy in tabs and made node type the default

CommentFileSizeAuthor
#23 fullcalendar-colors-1130092-22.patch2.81 KBaspilicious
#21 fullcalendar-colors-1130092-20.patch2.87 KBaspilicious
#16 problem.png59.29 KBgeerlingguy
#13 FCC-default-color-1130092-12.patch1.59 KBgeerlingguy
#10 fullcalendar-colors-1130092-10.diff10.11 KBaspilicious
#8 fullcalendar-colors-1130092-8.patch5.22 KBaspilicious
#6 fullcalendar-colors-1130092-6.patch5.22 KBtim.plunkett
#5 fullcalendar-colors-1130092-5.patch9.19 KBaspilicious
#3 fullcalendar_colors-1130092-3.patch5.63 KBtim.plunkett
fullcalendar_color.patch13.91 KBaspilicious

Comments

geerlingguy’s picture

geerlingguy commented

Oh hai!

Sorry I've been negligent over here. Past few weeks I've been working on a major transition with my employment... meaning my Drupal contrib time has gone from full steam ahead to full stop. But that will be changing in a week or two :-)

This is the first patch I've seen in an issue queue since the great Git migration... what's the best way to proceed in merging it (or is this already put into the main FullCalendar project?)?.

geerlingguy’s picture

geerlingguy commented
Status: Active » Fixed

Commited: http://drupalcode.org/sandbox/geerlingguy/1102994.git/commit/51e611e

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Fixed » Needs review
StatusFileSize
new fullcalendar_colors-1130092-3.patch5.63 KB

There was a bunch of cruft in that patch, this should fix it.

geerlingguy’s picture

geerlingguy commented

Cool. Will commit once I'm off this work computer, which blocks git (grr...).

The moral of this story is: if you want something done, get aspilicious and tim.plunkett involved. Major kudos to you guys!

aspilicious’s picture

aspilicious commented
StatusFileSize
new fullcalendar-colors-1130092-5.patch9.19 KB

WAIT...

I have a patch rdy with these fixes + taxonomy color integration rdy.
I couldn't manage to split those up.
So it would be nice if someone can give this a review.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new fullcalendar-colors-1130092-6.patch5.22 KB

Okay aspilicious and I worked this out on IRC a bit.

My patch #3 goes in first, and this one follows it up. Credit aspilicious on the second one, they're separate.

geerlingguy’s picture

geerlingguy commented

Sounds good! I'll try to get to this early in the afternoon...

aspilicious’s picture

aspilicious commented
StatusFileSize
new fullcalendar-colors-1130092-8.patch5.22 KB

A new patch, fixed some stuff. And made some changes after some irc talking.

Still: First apply 3 than this one

geerlingguy’s picture

geerlingguy commented
Status: Needs review » Needs work

@aspilicious / #8 - Can't apply that patch, for some reason, after having applied #3 patch (which is now in the repository, if you want to update code from that, and reapply the patch):

$ git apply fullcalendar-colors-1130092-8.patch
error: patch failed: fullcalendar_colors.module:54
error: fullcalendar_colors.module: patch does not apply

Or... is there some other way I should be applying this patch?

aspilicious’s picture

aspilicious commented
StatusFileSize
new fullcalendar-colors-1130092-10.diff10.11 KB

This should do it.
And I sneaked user role coloring into this :)

aspilicious’s picture

aspilicious commented
Status: Needs work » Needs review
geerlingguy’s picture

geerlingguy commented

I've applied the patch, and it seems to do most everything well. One question: It seems that the class to be added for an 'article' would be "node-type-article", but in my outputted HTML, I'm getting the class "article" instead. I cleared caches on the performance page. Just realized I'm still running alpha-3 of FullCalendar, which doesn't have the hook you're using to create the CSS. I'll install the latest dev release and see if that works.

geerlingguy’s picture

geerlingguy commented
StatusFileSize
new FCC-default-color-1130092-12.patch1.59 KB

Here's what I'm getting in the <head> section of my page, after having set some colors (for every entity type I could):

<style type="text/css" media="all"> 
<!--/*--><![CDATA[/*><!--*/
.node-type-article{background-color:#ffffff;border-color:#ffffff;}.node-type-article a{background-color:#ffffff;border-color:#ffffff;}.taxonomy-term-2{background-color:#222ad3;border-color:#222ad3;}.taxonomy-term-2 a{background-color:#222ad3;border-color:#222ad3;}.taxonomy-term-3{background-color:#ffffff;border-color:#ffffff;}.taxonomy-term-3 a{background-color:#ffffff;border-color:#ffffff;}.taxonomy-term-1{background-color:#42855b;border-color:#42855b;}.taxonomy-term-1 a{background-color:#42855b;border-color:#42855b;}
.node-type-article{background-color:#ffffff;border-color:#ffffff;}.node-type-article a{background-color:#ffffff;border-color:#ffffff;}
.node-type-page{background-color:#b5b668;border-color:#b5b668;}.node-type-page a{background-color:#b5b668;border-color:#b5b668;}
 
/*]]>*/-->
</style>

It seems the CSS isn't applying for me, and I think it has to do with the CSS inheritance imposed by the FullCalendar module. The .fc-event-skin background color is what actually shows up for me, instead of the background color of my choosing. My background color (set through FullCalendar Colors) only shows for the top and bottom pixel border of the event.

Also, I realized I should be setting a more sane default (rather than #ffffff) for the default color, because the text defaults to white... meaning you can't see the event at all if you use the default! (Fixing that in attached patch, which has been committed).

geerlingguy’s picture

geerlingguy commented
Status: Needs review » Needs work

It looks like, in order to get the color to apply correctly to the entire event, we'll need to add the same value as we do to the 'entity-type-[whatever]' class to the .fc-event-skin class. (This dual-application of the color seems to be how FC creates its 'rounded border' look... I think on one of my sites, I wiped out that setup altogether, and just had one div for the event, and then used border-radius instead).

aspilicious’s picture

aspilicious commented
Status: Needs work » Needs review

Did you clear your caches? Because css in should override fullcalendar css

geerlingguy’s picture

geerlingguy commented
StatusFileSize
new problem.png59.29 KB

Yes, please see attached screen grab...

aspilicious’s picture

aspilicious commented

I rly can't reproduce... What version of fullcalendar are u using?

geerlingguy’s picture

geerlingguy commented

Dev release, April 19.

aspilicious’s picture

aspilicious commented

I mean version of fullcalender plugin (in the libraries folder)

geerlingguy’s picture

geerlingguy commented
Status: Needs review » Needs work

So, it turns out that the CSS styling/classes in the 1.5.x series of FullCalendar has been changed, and that is what was causing the issues. With 1.4.x, there was no .fc-event-skin class applied to the events, so the current version of this module works fine. With 1.5.x, that class is applied, and even though Webkit inspector tells me that class's background/border color is overridden by our own classes/colors, the main part of the event is colored the default FullCalendar blue.

aspilicious’s picture

aspilicious commented
Status: Needs work » Needs review
StatusFileSize
new fullcalendar-colors-1130092-20.patch2.87 KB

This should work on fullcalendar 1.4 and 1.5 WHOOOHOOW

geerlingguy’s picture

geerlingguy commented
Status: Needs review » Needs work

Couldn't apply the patch...

$ git apply fullcalendar-colors-1130092-20.patch
fullcalendar-colors-1130092-20.patch:17: trailing whitespace.
  $color = variable_get('fullcalendar_color_node_type_' . $entity->type, '#3366CC');
fullcalendar-colors-1130092-20.patch:18: trailing whitespace.
  _fullcalendar_colors_build_css_string($css, $class, $color);
...
error: patch failed: fullcalendar_colors.module:119
error: fullcalendar_colors.module: patch does not apply
aspilicious’s picture

aspilicious commented
StatusFileSize
new fullcalendar-colors-1130092-22.patch2.81 KB

This one?

geerlingguy’s picture

geerlingguy commented
Status: Needs work » Needs review

Yeah... had to do with the line endings. In the patch in #21, the line endings seemed to be windows line endings. I changed them to LF by re-saving in TextMate, and the patch applied correctly. It's the silliest things. Will review/commit now...

geerlingguy’s picture

geerlingguy commented
Status: Needs review » Fixed

Well, after some more testing (there was a slight issue with the 'a' needing a space before it), it looks like all the issues have been resolved. aspilicious and tim.plunkett: you rock! This is awesome.

geerlingguy’s picture

geerlingguy commented

Follow-up issue: #1134132: fc-event-time class is unstyled in Week/Day view.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Project: » FullCalendar
Version: » 7.x-2.x-dev
Component: Code » FullCalendar Colors

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.