Edgewall Software

Opened 9 years ago

Last modified 7 years ago

#12048 closed enhancement

[PATCH] Add scope to CSS rules in timeline.css — at Version 6

Reported by: Baptiste Mispelon <bmispelon@…> Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.5
Component: timeline Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fixed timeline CSS rules interfering with styling of custom themes.

API Changes:

Timeline CSS rules are scoped to the .timeline class.

Internal Changes:

Description

The way the CSS rules are declared in timeline.css makes it hard to integrate Trac into an existing design.

You can see an example in the footer of https://code.djangoproject.com/timeline (big gray rectangles).

I would propose to add a .timeline prefix to the rules in the stylesheet to avoid this kind of problem.

Change History (8)

by Baptiste Mispelon <bmispelon@…>, 9 years ago

Attachment: timeline-css-scoping.diff added

Git-format patch

comment:1 by Ryan J Ollos, 9 years ago

Milestone: 1.0.6
Owner: set to Ryan J Ollos
Status: newassigned

Thanks for the patch. I see no problem with applying some form of the patch for the upcoming release, but let's see what the other devs have to say.

A few comments:

  • Which version of Trac are you running on the Django site? The patch applies cleanly on the trunk, but not on 1.0-stable.
  • The nested CSS would be much easier to manage if we were using something like Sass to generate our CSS, something the Trac project should keep in mind for the future.
  • An alternative to using the class selector, we could use the #content selector. The reason I mention that is that equivalent changes could then be applied to report.css, roadmap.css, etc; resulting in a very consistent pattern that could be useful for sites that use a theme to make Trac appear nested in the page.

I'm glad to see the Django project still running Trac!

by Baptiste Mispelon <bmispelon@…>, 9 years ago

Patch for 1.0-stable branch

comment:2 by Baptiste Mispelon <bmispelon@…>, 9 years ago

Regarding your comments:

  • Django currently runs 1.0.5. I've attached a patch against the 1.0 branch as well.
  • A CSS pre-processor would probably be really useful but it's a major undertaking. As a point of reference, the djangoproject website now uses SASS with libsass [1] and I find it works quite well.
  • I haven't looked into some of the other CSS files but the reason I chose the .timeline prefix was that in case someone was relying on the global styling in their custom CSS, it would be easy to restore the old behavior by wrapping the site in a <div class="timelime">.

And yes, we're still running our Trac instance and are approaching 25000 tickets. Trac has its warts (integrating it with our custom CSS was hard and running it locally is quite tricky), it's still a very solid bug tracker and an integral part of our project.

Thanks!

[1] https://pypi.python.org/pypi/libsass/0.7.0

Last edited 9 years ago by Ryan J Ollos (previous) (diff)

comment:3 by Jun Omae, 9 years ago

That changes seem to be good. However, it would lead a bit of incompatibilities to plugins which use the stylesheet, e.g. th:source:ticketmodifiedfilesplugin/0.11/ticketmodifiedfiles/ticketmodifiedfiles.py@11457:37#L28.

I think we should apply it on trunk.

Last edited 9 years ago by Jun Omae (previous) (diff)

comment:4 by Ryan J Ollos, 9 years ago

Milestone: 1.0.61.1.5

Okay, let's apply to trunk.

The patch doesn't include .timeline in the following rules: trunk/trac/htdocs/css/timeline.css@13937:55-63,66-67,70-71,76,86,89-94,102,106,110#L54. Should we also scope those rules to .timeline for consistency, even though they already match to classes rather than just element type?

in reply to:  4 comment:5 by Jun Omae, 9 years ago

Replying to rjollos:

The patch doesn't include .timeline in the following rules: trunk/trac/htdocs/css/timeline.css@13937:55-63,66-67,70-71,76,86,89-94,102,106,110#L54. Should we also scope those rules to .timeline for consistency, even though they already match to classes rather than just element type?

Yeah, I think we should scope all rules in timeline.css to .timeline. I consider that changes would be able to merge the timeline.css to trac.css and reduce requests to improve performance.

comment:6 by Ryan J Ollos, 9 years ago

API Changes: modified (diff)
Release Notes: modified (diff)
Note: See TracTickets for help on using tickets.