Patch #33437

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

Added by Mizuki ISHIKAWA 6 months ago. Updated 5 months 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 6 months 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 5 months ago

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

Patch by Marius BALTEANU.

History

#1 Updated by Go MAEDA 6 months 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 6 months ago

  • Target version set to 4.2.0

Setting the target version to 4.2.0.

#3 Updated by Go MAEDA 6 months 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 6 months 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 5 months 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 5 months 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