-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Ionicons (symbols) to <l:Icon> and <l:task> components #6186
Changes from 16 commits
efd57ff
2e6c2d7
6994e51
ec2fb5b
2080aab
586c22a
88b61aa
dfc874d
7ad92f6
e989919
e4bd510
e3d77ac
c2e47cb
d8169ad
b869976
58936fe
e3c1004
f918424
013f733
9fed655
839d823
f476b72
f8a0ece
26ebfad
cc2a014
f6de539
0987321
5241717
4614b08
2aa2508
88699d6
8a55f17
c9bda44
e03f7d0
f4261b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,6 +161,7 @@ | |
import org.apache.commons.jexl.parser.ASTSizeFunction; | ||
import org.apache.commons.jexl.util.Introspector; | ||
import org.apache.commons.lang.StringUtils; | ||
import org.jenkins.ui.icon.Icon; | ||
import org.jenkins.ui.icon.IconSet; | ||
import org.jvnet.tiger_types.Types; | ||
import org.kohsuke.accmod.Restricted; | ||
|
@@ -2289,4 +2290,49 @@ public static boolean isContextMenuVisible(Action a) { | |
return true; | ||
} | ||
} | ||
|
||
public static Icon tryGetIcon(String iconGuess, JellyContext context) { | ||
if (iconGuess.startsWith("symbol-")) { | ||
return null; | ||
} | ||
|
||
StaplerRequest currentRequest = Stapler.getCurrentRequest(); | ||
currentRequest.getWebApp().getDispatchValidator().allowDispatch(currentRequest, Stapler.getCurrentResponse()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Caused SECURITY-2777 |
||
String rootURL = currentRequest.getContextPath(); | ||
Icon iconMetadata = IconSet.icons.getIconByClassSpec(iconGuess); | ||
|
||
if (iconMetadata == null) { | ||
// Icon could be provided as a simple iconFileName e.g. "settings.png" | ||
iconMetadata = IconSet.icons.getIconByClassSpec(IconSet.toNormalizedIconNameClass(iconGuess) + " icon-md"); | ||
} | ||
|
||
if (iconMetadata == null) { | ||
// Icon could be provided as an absolute iconFileName e.g. "/plugin/foo/abc.png" | ||
iconMetadata = IconSet.icons.getIconByUrl(iconGuess); | ||
} | ||
|
||
return iconMetadata; | ||
} | ||
|
||
public static String tryGetIconPath(String iconGuess, JellyContext context) { | ||
janfaracik marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (iconGuess.startsWith("symbol-")) { | ||
return iconGuess; | ||
} | ||
|
||
StaplerRequest currentRequest = Stapler.getCurrentRequest(); | ||
currentRequest.getWebApp().getDispatchValidator().allowDispatch(currentRequest, Stapler.getCurrentResponse()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Caused SECURITY-2777 |
||
String rootURL = currentRequest.getContextPath(); | ||
Icon iconMetadata = tryGetIcon(iconGuess, context); | ||
String iconSource = null; | ||
|
||
if (iconMetadata != null) { | ||
iconSource = iconMetadata.getQualifiedUrl(context); | ||
} | ||
|
||
if (iconMetadata == null) { | ||
iconSource = rootURL + (iconGuess.startsWith("/images/") || iconGuess.startsWith("/plugin/") ? getResourcePath() : "") + getResourcePath() + iconGuess; | ||
} | ||
|
||
return iconSource; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,6 +133,18 @@ public ContextMenu add(String url, String icon, String text, boolean post, boole | |
return this; | ||
} | ||
|
||
/** @since TODO */ | ||
public ContextMenu add(String url, String icon, String iconXml, String text, boolean post, boolean requiresConfirmation) { | ||
if (text != null && icon != null && url != null) { | ||
MenuItem item = new MenuItem(url, icon, text); | ||
item.iconXml = iconXml; | ||
item.post = post; | ||
item.requiresConfirmation = requiresConfirmation; | ||
items.add(item); | ||
} | ||
return this; | ||
} | ||
|
||
/** | ||
* Add a header row (no icon, no URL, rendered in header style). | ||
* | ||
|
@@ -268,6 +280,13 @@ class MenuItem { | |
@SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD", justification = "read by Stapler") | ||
public String icon; | ||
|
||
/** | ||
* Optional icon XML, if set it's used instead of @icon for the menu item | ||
*/ | ||
@Exported | ||
@SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD", justification = "read by Stapler") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the suppression now |
||
public String iconXml; | ||
timja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* True to make a POST request rather than GET. | ||
* @since 1.504 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,8 @@ | |
import org.apache.commons.io.IOUtils; | ||
import org.apache.commons.jelly.JellyContext; | ||
import org.apache.commons.lang.StringUtils; | ||
import org.kohsuke.accmod.Restricted; | ||
import org.kohsuke.accmod.restrictions.NoExternalUse; | ||
|
||
/** | ||
* An icon set. | ||
|
@@ -45,7 +47,7 @@ public class IconSet { | |
|
||
|
||
public static final IconSet icons = new IconSet(); | ||
private static final Map<String, String> IONICONS = new ConcurrentHashMap<>(); | ||
private static final Map<String, String> SYMBOLS = new ConcurrentHashMap<>(); | ||
|
||
private Map<String, Icon> iconsByCSSSelector = new ConcurrentHashMap<>(); | ||
private Map<String, Icon> iconsByUrl = new ConcurrentHashMap<>(); | ||
|
@@ -73,34 +75,36 @@ private static String prependTitleIfRequired(String icon, String title) { | |
return icon; | ||
} | ||
|
||
public static String getIonicon(String name, String title) { | ||
timja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (IONICONS.containsKey(name)) { | ||
String icon = IONICONS.get(name); | ||
return prependTitleIfRequired(icon, title); | ||
// for Jelly | ||
@Restricted(NoExternalUse.class) | ||
public static String getSymbol(String name, String title) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the code I'm pretty sure this won't work for plugins wanting to add their own symbol. Probably okay for this PR but should be addressed in a follow up |
||
if (SYMBOLS.containsKey(name)) { | ||
String symbol = SYMBOLS.get(name); | ||
return prependTitleIfRequired(symbol, title); | ||
} | ||
|
||
// Load icon if it exists | ||
InputStream inputStream = IconSet.class.getResourceAsStream("/images/ionicons/" + name + ".svg"); | ||
String ionicon = null; | ||
// Load symbol if it exists | ||
InputStream inputStream = IconSet.class.getResourceAsStream("/images/symbols/" + name + ".svg"); | ||
String symbol = null; | ||
|
||
try { | ||
if (inputStream != null) { | ||
ionicon = IOUtils.toString(inputStream, StandardCharsets.UTF_8); | ||
symbol = IOUtils.toString(inputStream, StandardCharsets.UTF_8); | ||
} | ||
} catch (IOException e) { | ||
// ignored | ||
} | ||
if (ionicon == null) { | ||
ionicon = PLACEHOLDER_SVG; | ||
if (symbol == null) { | ||
symbol = PLACEHOLDER_SVG; | ||
} | ||
|
||
ionicon = ionicon.replaceAll("(<title>)[^&]*(</title>)", "$1$2"); | ||
ionicon = ionicon.replaceAll("<svg", "<svg aria-hidden=\"true\""); | ||
ionicon = ionicon.replace("stroke:#000", "stroke:currentColor"); | ||
symbol = symbol.replaceAll("(<title>)[^&]*(</title>)", "$1$2"); | ||
symbol = symbol.replaceAll("<svg", "<svg aria-hidden=\"true\""); | ||
symbol = symbol.replace("stroke:#000", "stroke:currentColor"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sadly there isn't any support for plugins yet. For adding Symbols to Jenkins, it should just work by adding them to the |
||
|
||
IONICONS.put(name, ionicon); | ||
SYMBOLS.put(name, symbol); | ||
|
||
return prependTitleIfRequired(ionicon, title); | ||
return prependTitleIfRequired(symbol, title); | ||
} | ||
|
||
public IconSet addIcon(Icon icon) { | ||
|
@@ -517,24 +521,11 @@ private static void initializeSVGs() { | |
images.add("warning"); | ||
images.add("document-properties"); | ||
|
||
Map<String, String> materialIcons = new HashMap<>(); | ||
materialIcons.put("help", "svg-sprite-action-symbol.svg#ic_help_24px"); | ||
|
||
for (Map.Entry<String, String> size : sizes.entrySet()) { | ||
for (String image : images) { | ||
icons.addIcon(new Icon("icon-" + image + " " + size.getKey(), | ||
"svgs/" + image + ".svg", size.getValue())); | ||
} | ||
|
||
for (Map.Entry<String, String> imageEntry : materialIcons.entrySet()) { | ||
icons.addIcon(new Icon( | ||
"icon-" + imageEntry.getKey() + " " + size.getKey(), | ||
"material-icons/" + imageEntry.getValue(), | ||
size.getValue(), | ||
IconFormat.EXTERNAL_SVG_SPRITE | ||
) | ||
); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some code path
iconGuess
can be null, either needs a null check, or callers adjusted in jelly to prevent thisNoticed on the update center page when installing plugins: http://localhost:8080/updateCenter/