プロジェクト

全般

プロフィール

Vote #74600

完了

Use css pseudo-classes instead of cycle("odd", "even")

Admin Redmine さんが約4年前に追加. 約4年前に更新.

ステータス:
Closed
優先度:
通常
担当者:
-
カテゴリ:
Code cleanup/refactoring_30
対象バージョン:
開始日:
2022/05/09
期日:
進捗率:

0%

予定工数:
category_id:
30
version_id:
119
issue_org_id:
15361
author_id:
68541
assigned_to_id:
1
comments:
17
status_id:
5
tracker_id:
2
plus1:
0
affected_version:
closed_on:
affected_version_id:
ステータス-->[Closed]

説明

I'm not sure it's a right way to ask such questions, so I'm sorry in advance.

I'm trying to produce some solution for making drag'n'drop in tables possible (see #12909).

I have a problem with @class="<%= cycle("odd", "even") %>">@, static classes are not suitable for drag'n'drop.

What do you think about using @:odd@ and @:even@ css pseudo-classes just to avoid static classes? I can make a patch, but will it be accepted?


journals

You should be able to reassign the appropriate css class to each row after drag'n'drop easily using javascript. But using pseudo-classes seems an interesting option to clean up the views.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
I've created a patch that replaces the cycle("odd", "even") with CSS pseudo-classes. The patch passes the tests.

Some remarks regarding this patch:
# In current version, roadmap and version pages don't have the odd/even classes, but with this patch will have the styles (which is good for consistency in my opinion).
# To keep the current style of block "Spent Time" from My Page (total row with "odd" style and time entries with white background), it was required to add two new CSS rules to specifically target that rows.
# For drag'n'drop feature (Redmine 3.3.0 #12909) it is no longer required to recalculate the odd/even classes using JS.
--------------------------------------------------------------------------------
Although the patch may break some themes, it makes view files cleaner and can save some processor cycles on web servers.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
Updated the patch to cleanly apply on the current trunk and also to include the new "cycle("odd", "even")" added in r15466
--------------------------------------------------------------------------------
I've committed the patch but it comes with an undesirable change: when displaying issue descriptions on the issue list, we used to get the issue row and its description with the same background color. Now, the issue and the description have their own background with different colors. It kind of breaks the readability of the list. Not sure if we can fix it using CSS only.
--------------------------------------------------------------------------------
Jean-Philippe Lang wrote:
> I've committed the patch but it comes with an undesirable change: when displaying issue descriptions on the issue list, we used to get the issue row and its description with the same background color. Now, the issue and the description have their own background with different colors. It kind of breaks the readability of the list. Not sure if we can fix it using CSS only.

I'll come up with a solution to fix this issue.
--------------------------------------------------------------------------------
Themes that colorize issues based on odd/even classes will have to get fixed (including the "alternate" theme provided with the core). Is the removal of 2 javascript lines worth this pain? I mean, what is the real benefit of using the CSS pseudo classes?
--------------------------------------------------------------------------------
FTR, I've reverted the changes until we find a solution for note-7 and take the decision to integrate the change.
--------------------------------------------------------------------------------
I've attached a new patch that fixes the issue from note-7. The patch includes both revisions (r16050 and r16051).

!issues_list.png!

The simplest solution found was to keep the odd/even classes in the "issues/_list.html.erb" and "timelogs/_list.html.erb" views. The fix required also to have a new class named "odd-even" for the tables that still use the odd/even classes.
--------------------------------------------------------------------------------
Jean-Philippe Lang wrote:
> Themes that colorize issues based on odd/even classes will have to get fixed (including the "alternate" theme provided with the core). Is the removal of 2 javascript lines worth this pain? I mean, what is the real benefit of using the CSS pseudo classes?

The new patch solves also the "alternate" theme. For other themes, the required fix is very small.

Regarding the real benefits, from my point of view:
- cleaner view files
- using a native and client-side only solution instead one that requires server-side processing and client-side
- simple way to style the tables (if you have a simple table, you only need the "list" class on the table; if you have a more complex table and you want to control the odd and even rows, you need the "list odd-even" classes and the "cycle" implementation).

For us, this feature is not a big improvement because in our custom theme we use the same color for rows and a bottom border as delimiter.
--------------------------------------------------------------------------------
Patch is committed, thanks Marius. I still had to fix the backgrounds for the alternate theme.
--------------------------------------------------------------------------------
Jean-Philippe Lang wrote:
> Patch is committed, thanks Marius. I still had to fix the backgrounds for the alternate theme.

I'm sure that I looked into the problem. I'll apply again the patch without your fix to understand what was the problem. Thanks.
--------------------------------------------------------------------------------
I still can't figure it out which was the problem with the alternate theme, I miss something, for sure. But is not so important.

r16249 adds the :not(.odd-even) to the respective selectors and make them more powerfull than the rule "div.mypage-box table.time-entries tr.time-entry". The attached patch fixes the issue generated by that change and keeps the current style of the block "Spent Time" from My Page.
--------------------------------------------------------------------------------
Reopening to handle #15361#note-15.
--------------------------------------------------------------------------------
Patch applied, thanks.
--------------------------------------------------------------------------------

Admin Redmine さんが約4年前に更新

  • カテゴリCode cleanup/refactoring_30 にセット
  • 対象バージョン3.4.0_119 にセット

他の形式にエクスポート: Atom PDF

いいね!0
いいね!0