-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
Expose org.jenkins.ui.symbol.Symbol
#6659
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
Merged
NotMyFault
merged 20 commits into
jenkinsci:master
from
Vlatombe:make-symbol-helper-public
Dec 20, 2022
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
9c082fc
Expose org.jenkins.ui.symbol.Symbol
Vlatombe f3146da
Fixes JENKINS-68805 and pick up #6685
Vlatombe b4db987
Merge branch 'master' into make-symbol-helper-public
Vlatombe f92ccaa
Clean up
Vlatombe a86eb39
Rename method since the intent slightly changed
Vlatombe 9c6d7ca
Simplify
Vlatombe 52f184e
Merge branch 'master' into make-symbol-helper-public
Vlatombe 676c4e8
Merge branch 'master' into make-symbol-helper-public
Vlatombe 828a1b0
Make this compile
daniel-beck 719ef97
Merge branch 'master' into make-symbol-helper-public
daniel-beck 0be317c
Remove support for legacy names
Vlatombe 7bc77b5
Fix reviews
Vlatombe f2be4c9
Merge branch 'master' into make-symbol-helper-public
basil 515b6bd
Merge branch 'master' into make-symbol-helper-public
basil b5570c0
Merge remote-tracking branch 'origin/master' into make-symbol-helper-…
basil f41e7fd
Add assertions for the symbol content used for testing
Vlatombe bcfe104
Merge branch 'master' into make-symbol-helper-public
Vlatombe 695e7be
Read science svg from filesystem
Vlatombe 1fa0f3f
Add withRaw to parse a raw selector
Vlatombe 5526933
Merge branch 'master' into make-symbol-helper-public
Vlatombe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| package org.jenkins.ui.symbol; | ||
|
|
||
| import edu.umd.cs.findbugs.annotations.CheckForNull; | ||
| import edu.umd.cs.findbugs.annotations.NonNull; | ||
| import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; | ||
| import hudson.Functions; | ||
| import hudson.PluginWrapper; | ||
| import hudson.Util; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import jenkins.model.Jenkins; | ||
| import org.apache.commons.io.IOUtils; | ||
| import org.apache.commons.lang.StringUtils; | ||
|
|
||
| /** | ||
| * Helper class to load symbols from Jenkins core or plugins. | ||
| * @since TODO | ||
| */ | ||
| public final class Symbol { | ||
| private static final Logger LOGGER = Logger.getLogger(Symbol.class.getName()); | ||
| // keyed by plugin name / core, and then symbol name returning the SVG as a string | ||
| private static final Map<String, Map<String, String>> SYMBOLS = new ConcurrentHashMap<>(); | ||
| static final String PLACEHOLDER_SVG = | ||
| "<svg xmlns=\"http://www.w3.org/2000/svg\" class=\"ionicon\" height=\"48\" viewBox=\"0 0 512 512\"><title>Close</title><path fill=\"none\" stroke=\"currentColor\" stroke-linecap=\"round\" stroke-linejoin=\"round\" stroke-width=\"32\" d=\"M368 368L144 144M368 144L144 368\"/></svg>"; | ||
|
|
||
| /** | ||
| * A substring of the placeholder so that tests can match it. | ||
| */ | ||
| static final String PLACEHOLDER_MATCHER = "M368 368L144 144M368 144L144 368"; | ||
|
|
||
| private Symbol() {} | ||
|
|
||
| /** | ||
| * Generates the svg markup for the given symbol name and attributes. | ||
| * @param request the symbol request object. | ||
| * @return The svg markup for the symbol. | ||
| * @since TODO | ||
| */ | ||
| public static String get(@NonNull SymbolRequest request) { | ||
| String name = request.getName(); | ||
| String title = request.getTitle(); | ||
| String tooltip = request.getTooltip(); | ||
| String htmlTooltip = request.getHtmlTooltip(); | ||
| String classes = request.getClasses(); | ||
| String pluginName = request.getPluginName(); | ||
| String id = request.getId(); | ||
|
|
||
| String identifier = StringUtils.defaultIfBlank(pluginName, "core"); | ||
|
|
||
| String symbol = SYMBOLS | ||
| .computeIfAbsent(identifier, key -> new ConcurrentHashMap<>()) | ||
| .computeIfAbsent(name, key -> loadSymbol(identifier, key)); | ||
| if (StringUtils.isNotBlank(tooltip) && StringUtils.isBlank(htmlTooltip)) { | ||
| symbol = symbol.replaceAll("<svg", "<svg tooltip=\"" + Functions.htmlAttributeEscape(tooltip) + "\""); | ||
| } | ||
| if (StringUtils.isNotBlank(htmlTooltip)) { | ||
| symbol = symbol.replaceAll("<svg", "<svg data-html-tooltip=\"" + Functions.htmlAttributeEscape(htmlTooltip) + "\""); | ||
| } | ||
| if (StringUtils.isNotBlank(id)) { | ||
| symbol = symbol.replaceAll("<svg", "<svg id=\"" + Functions.htmlAttributeEscape(id) + "\""); | ||
| } | ||
| if (StringUtils.isNotBlank(classes)) { | ||
| symbol = symbol.replaceAll("<svg", "<svg class=\"" + Functions.htmlAttributeEscape(classes) + "\""); | ||
| } | ||
| if (StringUtils.isNotBlank(title)) { | ||
| symbol = "<span class=\"jenkins-visually-hidden\">" + Util.xmlEscape(title) + "</span>" + symbol; | ||
| } | ||
| return symbol; | ||
| } | ||
|
|
||
|
|
||
| @SuppressFBWarnings(value = {"NP_LOAD_OF_KNOWN_NULL_VALUE", "RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE"}, justification = "Spotbugs doesn't grok try-with-resources") | ||
| private static String loadSymbol(String namespace, String name) { | ||
| String markup = PLACEHOLDER_SVG; | ||
| ClassLoader classLoader = getClassLoader(namespace); | ||
| if (classLoader != null) { | ||
| try (InputStream inputStream = classLoader.getResourceAsStream("images/symbols/" + name + ".svg")) { | ||
| if (inputStream != null) { | ||
| markup = IOUtils.toString(inputStream, StandardCharsets.UTF_8); | ||
| } else { | ||
| LOGGER.log(Level.FINE, "Missing symbol " + name + " in " + namespace); | ||
| } | ||
| } catch (IOException e) { | ||
| LOGGER.log(Level.FINE, "Failed to load symbol " + name, e); | ||
| } | ||
| } | ||
| return markup.replaceAll("(<title>).*?(</title>)", "$1$2") | ||
| .replaceAll("<svg", "<svg aria-hidden=\"true\"") | ||
| .replaceAll("(class=\").*?(\")", "") | ||
| .replaceAll("(tooltip=\").*?(\")", "") | ||
| .replaceAll("(data-html-tooltip=\").*?(\")", "") | ||
| .replaceAll("(id=\").*?(\")", "") | ||
| .replace("stroke:#000", "stroke:currentColor"); | ||
| } | ||
|
|
||
| @CheckForNull | ||
| private static ClassLoader getClassLoader(@NonNull String pluginName) { | ||
| if ("core".equals(pluginName)) { | ||
| return Symbol.class.getClassLoader(); | ||
| } else { | ||
| PluginWrapper plugin = Jenkins.get().getPluginManager().getPlugin(pluginName); | ||
| if (plugin != null) { | ||
| return plugin.classLoader; | ||
| } else { | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
@NotMyFault might this be the root cause of this PR instead? #6686
replace instead of replaceAll?
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.
The dot of the price tag has no stroke defined, this replacement pattern wouldn't apply at all.
Uh oh!
There was an error while loading. Please reload this page.
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.
replacereplaces all, it's just not a regex. (Although the first arg seems to assume it's never#000000? Is that not a potential problem?)Uh oh!
There was an error while loading. Please reload this page.
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.
I see that 3 core symbols have
stroke:#000(lock-closed.svg, play.svg and power.svg)And it appears twice in
lock-closed.svgandpower.svgso this could be updated. Not sure what is best between just fixing the svg files compared to changing this code.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.
ah my bad.
it could be yes although I guess it's an assumption based on what ionicon's does
Uh oh!
There was an error while loading. Please reload this page.
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.
Looks like everything is fine so far. However having some logic to validate symbols at build time would be interesting.
What comes to my mind:
pathhas at least one offill,strokedefined and set to eithernoneorcurrentColor. Could be defined either through attribute or throughstyle.