Integrate with Prism API plugin #72
Conversation
timja
force-pushed
the
prism-api-integration
branch
from
9a15fc1
to
faba94e
Compare
| <script type="text/javascript" src="${resURL}/plugin/prism-api/js/prism-clike.js"/> | ||
| <script type="text/javascript" src="${resURL}/plugin/prism-api/js/prism-css.js"/> | ||
| <script type="text/javascript" src="${resURL}/plugin/prism-api/js/prism-java.min.js"/> | ||
| <link type="text/css" rel="stylesheet" href="${resURL}/plugin/prism-api/css/${it.themeCssFileName}"/> |
There was a problem hiding this comment.
@uhafner this caught me out, it's not defined in the prism.jelly but in the SourceCodeViewModel https://github.com/jenkinsci/prism-api-plugin/blob/main/src/main/resources/io/jenkins/plugins/prism/SourceCodeViewModel/index.jelly#L10
seemed a bit strange that the plugin didn't load it, some sort of invoke static could probably be used to avoid having to define the getter?
There was a problem hiding this comment.
Yes, it would make sense to autoload the theme somehow in the main jelly file. Since the warnings plugin is the only component so far that uses the plugin it was not yet necessary up to now. I'm not sure how to store that information on the JS side though. I think when we make the theming customizable via the theme manager plugin we should change that...
There was a problem hiding this comment.
timja
force-pushed
the
prism-api-integration
branch
from
b65e1db
to
1a3d1e6
Compare
timja
force-pushed
the
prism-api-integration
branch
from
953c53a
to
c529b66
Compare
timja
force-pushed
the
prism-api-integration
branch
from
c529b66
to
a93a078
Compare
| <!-- For some reason need to explicitly depend on commons-text-api and commons-lang3-api for plugin-util-api to load --> | ||
| <dependency> | ||
| <groupId>io.jenkins.plugins</groupId> | ||
| <artifactId>commons-text-api</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.jenkins.plugins</groupId> | ||
| <artifactId>commons-lang3-api</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
When I use prism 1.29.0-8 as local dependency I do not see such a problem. Where exactly did you see the problem? I've seen that in the BOM builds Mark also had this problem.
| <!-- For some reason need to explicitly depend on commons-text-api and commons-lang3-api for plugin-util-api to load --> | |
| <dependency> | |
| <groupId>io.jenkins.plugins</groupId> | |
| <artifactId>commons-text-api</artifactId> | |
| </dependency> | |
| <dependency> | |
| <groupId>io.jenkins.plugins</groupId> | |
| <artifactId>commons-lang3-api</artifactId> | |
| </dependency> |
There was a problem hiding this comment.
unsure but lots of people were complaining about it for checks-api, and I added them there too:
jenkinsci/checks-api-plugin#233
Although at least in checks api it actually used those dependencies.
There was a problem hiding this comment.
This fails injected tests from maven hpi plugin, if you try locally you will get:
[ERROR] Failures:
[ERROR] InjectedTest.testPluginActive While testing design-library, plugin-util-api failed to start
Caused by: java.io.IOException: Failed to load: Plugin Utilities API Plugin (plugin-util-api 3.3.0)
- Plugin is missing: commons-text-api (1.10.0-36.vc008c8fcda_7b_)
- Plugin is missing: commons-lang3-api (3.12.0-36.vd97de6465d5b_)
at hudson.PluginWrapper.resolvePluginDependencies(PluginWrapper.java:988)
at hudson.PluginManager$2$1$1.run(PluginManager.java:555)
at org.jvnet.hudson.reactor.TaskGraphBuilder$TaskImpl.run(TaskGraphBuilder.java:177)
at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:305)
at jenkins.model.Jenkins$5.runTask(Jenkins.java:1170)
at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:221)
at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:120)
at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
| window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', event => { | ||
| setPrismBackgroundVariable() | ||
| }); | ||
| } |
There was a problem hiding this comment.
Wouldn't it be sufficient to call Prism.highlightElement(element) when the user changes the theme?
There was a problem hiding this comment.
No I've added some comments to explain more but the above isn't for the elements it's so that the copy to clipboard section matches which doesn't use prism:
Co-authored-by: Ullrich Hafner <ullrich.hafner@gmail.com>
…lugin into prism-api-integration
| // On the inputs page the preview markup link adds a hash to the url which breaks the regex extraction | ||
| const strippedHash = window.location.href.replace('#', '') | ||
| const componentName = strippedHash.match(/.+design-library\/(.+)$/)[1] |
There was a problem hiding this comment.
Minor bug introduced in #281 cc @mawinter69
at least locally in hpi:run I can't reproduce it on weekly.ci.jenkins.io
If I click it locally I get a # fragment added to the url and the page scrolled to the top and then if I refresh the page none of the examples load
On weekly it live updates in place though with no issue
Fixes #68
Requires jenkinsci/prism-api-plugin#107
I've tested this pretty heavily, it was quite fiddly but as far as I can tell it's working nicely
Testing done
By going to the design library views and clicking through them
Note: Design library is quite a complex integration for this as it loads sources dynamically after Prism runs in some cases so it has some extra hardening to make it more reliable, simple integrations should be a lot easier
Tested with:
Todo:
Split prism api into multiple plugins, it has too many dependencies and blocks things like Look at moving to prism api plugin for view export page configuration-as-code-plugin#1835- bootstrap5-api has a very high install count, it's fine to leave as is I thinkCurrently you need to select the theme in your system configuration, screenshot is using 'Tomorrow Night'
cc @janfaracik / @uhafner