Patch #33437

Add missing icon class to items with icon-checked class in the context menu

Added by Mizuki ISHIKAWA 18 days ago. Updated 12 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Issues
Target version:4.2.0

Description

Neither .icon class nor .icon-only class exists in context menu link with green check icon.
It looks fine, but inconsistent compared to other icon links.

There is a problem when changing the icon style depending on the theme, so please fix it.


diff --git a/app/helpers/context_menus_helper.rb b/app/helpers/context_menus_helper.rb
index db71abe31..d545f3c51 100644
--- a/app/helpers/context_menus_helper.rb
+++ b/app/helpers/context_menus_helper.rb
@@ -21,7 +21,7 @@ module ContextMenusHelper
   def context_menu_link(name, url, options={})
     options[:class] ||= ''
     if options.delete(:selected)
-      options[:class] += ' icon-checked disabled'
+      options[:class] += ' icon icon-checked disabled'
       options[:disabled] = true
     end
     if options.delete(:disabled)

ScreenShot.png (163 KB) Mizuki ISHIKAWA, 2020-05-13 07:32


Related issues

Related to Redmine - Patch #28605: Add the missing icon class to the items with icons from t... Closed

Associated revisions

Revision 19778
Added by Go MAEDA 16 days ago

Add missing "icon" class to items with "icon-checked" class in the context menu (#33437).

Patch by Mizuki ISHIKAWA.

Revision 19782
Added by Go MAEDA 12 days ago

Remove existing rules made unnecessary by r19778 (#33437).

Patch by Marius BALTEANU.

History

#1 Updated by Go MAEDA 18 days ago

  • Related to Patch #28605: Add the missing icon class to the items with icons from the contextual menu added

#2 Updated by Go MAEDA 18 days ago

  • Target version set to 4.2.0

Setting the target version to 4.2.0.

#3 Updated by Go MAEDA 16 days ago

  • Subject changed from Add missing .icon classes to links in context menu to Add missing icon class to items with icon-checked class in the context menu
  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed the fix. Thank you.

#4 Updated by Marius BALTEANU 13 days ago

  • Status changed from Closed to Reopened

We should clean-up the existing rule:

mariusbalteanu@Mariuss-MacBook-Pro redmine % git diff
diff --git a/public/stylesheets/context_menu.css b/public/stylesheets/context_menu.css
index ff83f60a1..efa25006a 100644
--- a/public/stylesheets/context_menu.css
+++ b/public/stylesheets/context_menu.css
@@ -46,7 +46,7 @@
 #context-menu li.folder:hover { z-index:40; }
 #context-menu ul ul, #context-menu  li:hover ul ul { display:none; }
 #context-menu li:hover ul, #context-menu li:hover li:hover ul { display:block; }
-#context-menu a.icon-checked {background: url(../images/toggle_check.png) no-repeat 3px 40%;}
+#context-menu a.icon-checked {background-position: 3px 40%;}

 /* selected element */
 .context-menu-selection { background-color:#507AAA !important; color:#f8f8f8 !important; }

#5 Updated by Mizuki ISHIKAWA 13 days ago

Marius BALTEANU wrote:

We should clean-up the existing rule:

[...]

I didn't realize there is a style for context menus only.
Thank you for reviewing!

#6 Updated by Go MAEDA 12 days ago

  • Status changed from Reopened to Closed

Marius BALTEANU wrote:

We should clean-up the existing rule:

Committed the fix in r19782. Thank you.

Also available in: Atom PDF