Patch #24313

Use the regular "icon icon-*" classes for all elements with icons

Added by Marius BALTEANU 7 months ago. Updated 7 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Code cleanup/refactoring
Target version:3.4.0

Description

Working at #23980 we discovered that some links or other elements with icons (background image) doesn't use the regular "icon icon-*" classes.

I think that for consistency, better theme support and easier styling, we should add the "icon icon-*" also to these elements.

The attached patch is a first patch that implements this for admin menu links.

01_user_regular_icons_for_admin_links.patch Magnifier (5.62 KB) Marius BALTEANU, 2016-11-09 22:27

02_use_regular_icons_in_overview_tab.patch Magnifier (3.59 KB) Marius BALTEANU, 2016-11-10 01:09

03_use_regular_icons_in_activity_tab.patch Magnifier (6.91 KB) Marius BALTEANU, 2016-11-10 01:09

04_use_regular_icons_in_version_tab.patch Magnifier (1.75 KB) Marius BALTEANU, 2016-11-10 01:09

05_use_regular_icons_in_search_results.patch Magnifier (10.1 KB) Marius BALTEANU, 2016-11-12 14:49

07_small_improvement.patch Magnifier (1.33 KB) Marius BALTEANU, 2016-11-12 14:49

06_other_pages.patch Magnifier (15 KB) Marius BALTEANU, 2016-11-12 14:49

02_use_regular_icons_in_overview_tab_v2.patch Magnifier (3.58 KB) Marius BALTEANU, 2016-11-12 15:06

05_use_regular_icons_in_search_results_v2.patch Magnifier (1.9 KB) Marius BALTEANU, 2016-11-18 18:50

Associated revisions

Revision 15972
Added by Jean-Philippe Lang 7 months ago

Use the regular "icon icon-*" for admin menu links (#24313).

Patch by Marius BALTEANU.

Revision 15973
Added by Jean-Philippe Lang 7 months ago

Use the regular "icon icon-*" on project overview (#24313).

Patch by Marius BALTEANU.

Revision 15975
Added by Jean-Philippe Lang 7 months ago

Use the regular "icon icon-*" on activity view (#24313).

Patch by Marius BALTEANU.

Revision 15976
Added by Jean-Philippe Lang 7 months ago

Use the regular "icon icon-*" on roadmap (#24313).

Patch by Marius BALTEANU.

Revision 15977
Added by Jean-Philippe Lang 7 months ago

Use the regular "icon icon-*" on search results (#24313).

Revision 15978
Added by Jean-Philippe Lang 7 months ago

Use the regular "icon icon-*" in other pages (#24313).

Revision 15979
Added by Jean-Philippe Lang 7 months ago

Small improvement (#24313).

Patch by Marius BALTEANU.

Revision 15980
Added by Jean-Philippe Lang 7 months ago

Fix icon-news class (#24313).

History

#1 Updated by Marius BALTEANU 7 months ago

Another patches for overview, activity and roadmap tabs.

#2 Updated by Marius BALTEANU 7 months ago

Added the last patches:

- 05_use_regular_icons_in_search_results.patch:

The change from the view (app/views/search/index.html) was required because the classes in the search page are delimited by space and not by "-" like in the activity page even if are the same events. I think that this fix should be made in the Search and libs modules in order to return the same event_type for closed issues, but I wasn't able to do it.

event type: closed issue
class in activity: "issue-closed"
class in search: "issue closed"

- 06_other_pages.patch

Changes in various pages: like favorite projects, members, attachments, forums.

- 07_small_improvement.patch:

The class "icon" should not have the rules with padding-top and padding-bottom because these are required only when the font size is smaller than the default one (like is in the contextual block or calendar).

- Updated an already uploaded patch to use the background-image rule and not just the background rule.

- There are some rules in the CSS file that I wasn't able to find them in the application:

table.boards a.board { background: url(../images/comment.png) no-repeat 0% 50%; padding-left: 20px; }
span.add_attachment a {padding-left:16px; background: url(../images/bullet_add.png) no-repeat 0 50%; }
.task_late { background:#f66 url(../images/task_late.png); border: 1px solid #f66; }
.task_done { background:#00c600 url(../images/task_done.png); border: 1px solid #00c600; }
.task_todo { background:#aaa url(../images/task_todo.png); border: 1px solid #aaa; }
... 
.project.task_late { background:#f66 url(../images/milestone_late.png); border: 1px solid #f66; height: 2px; margin-top: 3px;}
.project.task_done { background:#00c600 url(../images/milestone_done.png); border: 1px solid #00c600; height: 2px; margin-top: 3px;}
.project.task_todo { background:#fff url(../images/milestone_todo.png); border: 1px solid #fff; height: 2px; margin-top: 3px;}
.project.marker { background-image:url(../images/project_marker.png); background-repeat: no-repeat; border: 0; margin-left: -4px; margin-top: 1px; }

Any feedback is welcome on this issue.

#3 Updated by Jean-Philippe Lang 7 months ago

  • Target version set to 3.4.0

#4 Updated by Jean-Philippe Lang 7 months ago

Activity view: event type class preserved in addition to the icon-* class.

#5 Updated by Marius BALTEANU 7 months ago

Jean-Philippe Lang wrote:

Activity view: event type class preserved in addition to the icon-* class.

I've updated the patch 05_use_regular_icons_in_search_results.patch in order to preserve the event type class like you made in the activity.

Thanks for committing this patch so fast.

#6 Updated by Jean-Philippe Lang 7 months ago

Marius BALTEANU wrote:

- 05_use_regular_icons_in_search_results.patch:

The change from the view (app/views/search/index.html) was required because the classes in the search page are delimited by space and not by "-" like in the activity page even if are the same events. I think that this fix should be made in the Search and libs modules in order to return the same event_type for closed issues, but I wasn't able to do it.

There was a difference between Issue and Journal acts_as_event definition. It's fixed and I've removed the #parameterize call from the patch.

#7 Updated by Jean-Philippe Lang 7 months ago

Patch 6 committed with several changes. I've added a few icon-* (eg. icon-sticky) classes that are more meaningful than icon-arrow-right, even if they actually use the same image in the default CSS.

#8 Updated by Jean-Philippe Lang 7 months ago

  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang

And I've hidden the user icon on the member list, just like it was before the patches.

All patches are now committed, thanks.

#9 Updated by Marius BALTEANU 7 months ago

Thanks Jean-Philippe Lang for your feedback on these patches.

I think that one more change is required, you committed the 02_use_regular_icons_in_overview_tab.patch and not the second version of the patch 02_use_regular_icons_in_overview_tab_v2.patch which contains a small fix for .icon-new class.

 
.icon-news { background: url(../images/news.png); }
 

should be
.icon-news { background-image: url(../images/news.png); }

#10 Updated by Jean-Philippe Lang 7 months ago

Marius BALTEANU wrote:

I think that one more change is required

Committed, thanks.

Also available in: Atom PDF