This issue is basically a code tidy-up.

If I understand correctly, the rrssb-overrides.css exists because the Drupal theme has a bunch of CSS relating to .item-list and this incorrectly overrides rrssb.css. So the overrides file repeats lines from rrssb.css but with extra qualifiers to make them take precedence. This seems a little untidy - potentially it depends on the theme exactly what needs to be repeated. Also it tends to get out of date easily - changes to the original rrssb.css need to be echoed in the override file. Right now, it looks like it is slightly out of date.

I have a patch that seems to be a neater approach: don't add the .item-list class in the first place and it all just works. However I have retained the overrides file with one new rule in it. With one theme I'm using (https://www.drupal.org/project/pixture_reloaded) the buttons stop being bold on hover. This last one is nothing to do with .item-list so I can't see a clever fix.

CommentFileSizeAuthor
#3 rrssb.item-list.2488360.3.patch1.36 KBAdamPS
#1 rrssb.item-list.2488360.1.patch1.42 KBAdamPS
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented
FileSize
rrssb.item-list.2488360.1.patch1.42 KB
mike.roberts’s picture

mike.roberts CreditAttribution: mike.roberts commented

That patch feels really dirty to me :-P. Is there a better way to do this?

AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented
FileSize
rrssb.item-list.2488360.3.patch1.36 KB

I can't see a better way. Options I can think of in order of preference in my view:

  1. Patch as supplied (and rerolled for latest dev). Yes, it's dirty but also quite cunning given that core doesn't provide a means to alter the class. It only adds a couple of lines of code, and they won't to be kept up to date.
  2. Copy the code out of theme_item_list altering the one problematic line (and perhaps discarding bits we don't need). Adds 10-30 lines, duplicating core, but probably doesn't need to be kept up to date as core isn't likely to change.
  3. Change nothing. Adds 10 lines of code (the CSS already present in theme override), it might not work for all themes, and we have to keep it up to date with the library changes.

So in the absence of a better idea, OK if I commit option 1?

mike.roberts’s picture

mike.roberts CreditAttribution: mike.roberts commented

Yeah I guess go ahead and commit it, and if a better option comes alone we'll add it later :)

  • AdamPS committed 8c7703e on 
    Issue #2488360 by AdamPS: Tidy up CSS overrides
AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented
Status: Needs review » Fixed
AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented

Hmm it became slightly more fiddly when I merged your latest change to call 

theme('item_list'

rather than 

theme_item_list

. The change is correct of course, but it means the resulting output is less predictable. Coming up is another commit that makes the code more flexible, but it isn't absolutely guaranteed to work. But then the original code wasn't absolutely guaranteed to work either.

Maybe we'll come up with something better in future. There is much activity on the RRSSB library including a version 2.

  • AdamPS committed 5e04b2a on 7.x-1.x 
    Issue #2488360 by AdamPS: Tidy up CSS overrides
- Make the string...
AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented

I have realised that the line I added to the overrides CSS was rather specific to one particular site/theme, so I will remove it - and hence remove the empty file.

  • AdamPS committed 2a038c1 on 7.x-1.x 
    Issue #2488360 by AdamPS: Tidy up CSS overrides
Remove line I added to...

Status: Fixed » Closed (fixed)

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