Skip to content
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 sidebar to plugin manager, increase search bar size #6783

Merged
merged 59 commits into from
Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
b8a622c
Init
janfaracik Apr 21, 2022
53902db
Update sidebar
janfaracik Apr 21, 2022
addb738
Finish
janfaracik Apr 21, 2022
cf28fee
Add loading animation
janfaracik Apr 21, 2022
84596ee
Update plugin-manager.less
janfaracik Apr 25, 2022
dc8ba52
Fix translucency on Firefox
janfaracik Apr 28, 2022
28aaf91
Remove unused icon preload
janfaracik Apr 28, 2022
1cb37d1
Revert "Remove unused icon preload"
janfaracik Apr 28, 2022
43e4f5c
Init
janfaracik May 16, 2022
d579dc5
Update section.less
janfaracik May 16, 2022
f89fe63
Merge branch 'master' into new-plugin-manager-1
janfaracik May 26, 2022
e925490
Add mixin for items
janfaracik Jun 8, 2022
8b22270
Update theme.less
janfaracik Jun 8, 2022
8c4b466
Merge branch 'master' into move-manage-jenkins-less
janfaracik Jun 8, 2022
929e2d5
Update mixins.less
janfaracik Jun 8, 2022
68f5425
Update theme.less
janfaracik Jun 8, 2022
13d5729
Move from px to rem
janfaracik Jun 9, 2022
d1f42ac
Update table.less
janfaracik Jun 10, 2022
28d0638
Merge branch 'master' into move-manage-jenkins-less
janfaracik Jun 14, 2022
90fc4ba
Rename mixin
janfaracik Jun 15, 2022
e21d4e7
Update breadcrumbs to use new item mixin
janfaracik Jun 15, 2022
69c1633
Update style.less
janfaracik Jun 16, 2022
e57bc28
Update theme.less
janfaracik Jun 16, 2022
0996e75
Update theme.less
janfaracik Jun 16, 2022
c78dfd6
Update theme.less
janfaracik Jun 16, 2022
c9e2823
Update colors
janfaracik Jun 16, 2022
2391319
Merge branch 'master' into move-manage-jenkins-less
janfaracik Jun 16, 2022
88b2a59
Merge branch 'move-manage-jenkins-less' into new-plugin-manager-1
janfaracik Jun 19, 2022
abcbecf
Update search bar
janfaracik Jun 19, 2022
87c27c2
Update side-panel-tasks.less
janfaracik Jun 20, 2022
a39862c
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 5, 2022
a688b3b
Restore tab bar
janfaracik Jul 5, 2022
a46beb7
Improve visibility of search bar
janfaracik Jul 5, 2022
9f3c69c
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 5, 2022
d74aeae
Remove unused page variable
janfaracik Jul 5, 2022
dbc96f4
Merge branch 'master' into new-plugin-manager-1
timja Jul 6, 2022
6e64c68
Merge branch 'master' into new-plugin-manager-1
timja Jul 6, 2022
db677f3
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 6, 2022
0f2ef2f
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 7, 2022
e14b7d2
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 12, 2022
4d640b4
Merge branch 'master' into new-plugin-manager-1
timja Jul 14, 2022
0b808b0
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 18, 2022
b8880ed
Fix checkbox controller selection
janfaracik Jul 18, 2022
09eb5f1
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 18, 2022
5976926
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 24, 2022
e8905ba
Small tab title update
janfaracik Jul 24, 2022
24aa2aa
Merge branch 'master' into new-plugin-manager-1
janfaracik Jul 24, 2022
ca35bde
Merge branch 'master' into new-plugin-manager-1
timja Aug 8, 2022
c5a6ad2
Merge branch 'master' into new-plugin-manager-1
timja Aug 15, 2022
e7fd7c2
Address most feedback
timja Aug 17, 2022
76ca102
Use right name
timja Aug 17, 2022
2beccbb
Adapt breadcrumbs
timja Aug 17, 2022
444170c
Merge branch 'master' into new-plugin-manager-1
timja Aug 17, 2022
2e199cc
Fix Downloads page title, remove separator styling
janfaracik Aug 17, 2022
bc516e1
Remove unused resource bundles and jelly files
timja Aug 18, 2022
7e71b5a
Use regular sidepanel
timja Aug 18, 2022
e4ae2c3
Merge remote-tracking branch 'origin/master' into new-plugin-manager-1
basil Aug 25, 2022
27b52cd
Merge branch 'master' into new-plugin-manager-1
basil Aug 26, 2022
1699d71
Merge branch 'master' into new-plugin-manager-1
NotMyFault Aug 28, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions core/src/main/java/hudson/PluginManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.HttpResponses;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerFallback;
import org.kohsuke.stapler.StaplerOverridable;
import org.kohsuke.stapler.StaplerProxy;
import org.kohsuke.stapler.StaplerRequest;
Expand Down Expand Up @@ -1386,6 +1387,27 @@ public static boolean isNonMetaLabel(String label) {
return !("adopt-this-plugin".equals(label) || "deprecated".equals(label));
}

/**
* This allows "Update Center" to live at the URL
* {@code /pluginManager/updates/} in addition to its {@code /updateCenter/}
* URL which is provided by {@link jenkins.model.Jenkins#getUpdateCenter()}.
* For purposes of Stapler, this object is the current item serving the
* view, and since this is not a {@link hudson.model.ModelObject}, it does
* not appear as an additional breadcrumb and only the "Plugin Manager"
* breadcrumb is shown.
*/
@Restricted(NoExternalUse.class)
public static class UpdateCenterProxy implements StaplerFallback {
@Override
public Object getStaplerFallback() {
return Jenkins.get().getUpdateCenter();
}
}

public UpdateCenterProxy getUpdates() {
return new UpdateCenterProxy();
}

@Restricted(NoExternalUse.class)
public HttpResponse doPluginsSearch(@QueryParameter String query, @QueryParameter Integer limit) {
List<JSONObject> plugins = new ArrayList<>();
Expand Down Expand Up @@ -1593,7 +1615,7 @@ public void doInstall(StaplerRequest req, StaplerResponse rsp) throws IOExceptio
boolean dynamicLoad = req.getParameter("dynamicLoad") != null;
install(plugins, dynamicLoad);

rsp.sendRedirect("../updateCenter/");
rsp.sendRedirect("updates/");
}

/**
Expand Down Expand Up @@ -1898,7 +1920,7 @@ public HttpResponse doUploadPlugin(StaplerRequest req) throws IOException, Servl
element("url", t.toURI().toString()).
element("dependencies", dependencies);
new UpdateSite(UpdateCenter.ID_UPLOAD, null).new Plugin(UpdateCenter.ID_UPLOAD, cfg).deploy(true);
return new HttpRedirect("../updateCenter");
return new HttpRedirect("updates/");
} catch (FileUploadException e) {
throw new ServletException(e);
}
Expand Down Expand Up @@ -2124,7 +2146,7 @@ public JSONArray doPrevalidateConfig(StaplerRequest req) throws IOException {
@RequirePOST
public HttpResponse doInstallNecessaryPlugins(StaplerRequest req) throws IOException {
prevalidateConfig(req.getInputStream());
return HttpResponses.redirectViaContextPath("updateCenter");
return HttpResponses.redirectViaContextPath("updates/");
}

/**
Expand Down
8 changes: 3 additions & 5 deletions core/src/main/resources/hudson/PluginManager/advanced.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,14 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout"
xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<l:layout title="${%Advanced Settings} - ${%Plugin Manager}" permission="${app.SYSTEM_READ}">
<l:layout title="${%Advanced settings} - ${%Plugin Manager}" permission="${app.SYSTEM_READ}">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Detail - Category a reasonable presentation universal across languages Jenkins is translated to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question and something I haven't got a concrete answer to. I've given it a quick Google but haven't found any articles on the topic, so I've looked at how various sites behave with right-to-left languages:

Google Search - Doesn't change the hierarchy of the title (e.g. Jenkins - Google)
Wikipedia - Does change the hierarchy of the title (e.g. Wikipedia - Jenkins)
Google Translate - Does change the hierarchy of the title

<j:set var="readOnlyMode" value="${!app.hasPermission(app.ADMINISTER)}"/>

<l:app-bar title="${%Plugin Manager}"/>
<l:app-bar title="${%Advanced settings}"/>

<st:include page="sidepanel.jelly"/>
<st:include page="sidepanel.jelly"/>

<l:main-panel>
<local:tabBar page="advanced" xmlns:local="/hudson/PluginManager"/>

<section class="jenkins-section jenkins-!-margin-bottom-5">
<h2 class="jenkins-section__title">${%HTTP Proxy Configuration}</h2>
<f:form method="post" action="proxyConfigure" name="proxyConfigure">
Expand Down
33 changes: 16 additions & 17 deletions core/src/main/resources/hudson/PluginManager/available.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,30 @@ THE SOFTWARE.
-->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout" xmlns:f="/lib/form">
<l:layout title="${%Available Plugins} - ${%Plugin Manager}" permission="${app.SYSTEM_READ}">
<j:set var="page" value="available"/>
<l:layout title="${%Available plugins} - ${%Plugin Manager}" permission="${app.SYSTEM_READ}">

<l:app-bar title="${%Plugin Manager}" />
<l:app-bar title="${%Plugins}" />

<st:include page="sidepanel.jelly"/>

<l:main-panel>
<script src="${resURL}/jsbundles/plugin-manager-ui.js" type="text/javascript"/>

<form method="post" action="install">
<local:tabBar page="${page}" xmlns:local="/hudson/PluginManager"/>

<div class="jenkins-form-item jenkins-search">
<input
class="jenkins-search__input"
type="search"
id="filter-box"
value="${request.getParameter('filter')}"
placeholder="${%Search}"/>
<div class="jenkins-search__icon">
<l:icon src="symbol-search"/>
<div class="jenkins-app-bar jenkins-app-bar--sticky">
<div class="jenkins-app-bar__content">
<div class="app-plugin-manager__search">
<span class="app-plugin-manager__search__icon">
<l:icon src="symbol-search"/>
</span>
<input type="search"
placeholder="Search available plugins"
id="filter-box"
value="${request.getParameter('filter')}" />
</div>
</div>
</div>

<script src="${resURL}/jsbundles/plugin-manager-ui.js" type="text/javascript"/>

<form method="post" action="install">
<table id="plugins" class="jenkins-table sortable"
data-hasAdmin="${h.hasPermission(app.ADMINISTER)}">
<thead>
Expand Down
32 changes: 16 additions & 16 deletions core/src/main/resources/hudson/PluginManager/installed.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,28 @@ THE SOFTWARE.
-->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout" xmlns:s="/lib/form">
<l:layout title="${%Installed Plugins} - ${%Plugin Manager}" permission="${app.SYSTEM_READ}">
<l:layout title="${%Installed plugins} - ${%Plugin Manager}" permission="${app.SYSTEM_READ}">
<j:set var="readOnlyMode" value="${!app.hasPermission(app.ADMINISTER)}"/>

<l:app-bar title="${%Plugin Manager}" />
<l:app-bar title="${%Plugins}" />

<st:include page="sidepanel.jelly"/>

<l:main-panel>
<div class="jenkins-app-bar jenkins-app-bar--sticky">
<div class="jenkins-app-bar__content">
<div class="app-plugin-manager__search">
<span class="app-plugin-manager__search__icon">
<l:icon src="symbol-search"/>
</span>
<input type="search"
placeholder="Search installed plugins"
id="filter-box"
value="${request.getParameter('filter')}" />
</div>
</div>
</div>

<st:adjunct includes="hudson.PluginManager._table"/>

<j:if test="${app.updateCenter.isRestartRequiredForCompletion()}">
Expand All @@ -53,20 +67,6 @@ THE SOFTWARE.
data-detached-possible-dependents="${%detached-possible-dependents}"
/>

<local:tabBar page="installed" xmlns:local="/hudson/PluginManager" />

<div class="jenkins-form-item jenkins-search">
<input
class="jenkins-search__input"
type="search"
id="filter-box"
value="${request.getParameter('filter')}"
placeholder="${filtered == 'true' ? '%Search' : '%Filter'}"/>
<div class="jenkins-search__icon">
<l:icon src="symbol-search"/>
</div>
</div>

<table id="plugins" class="jenkins-table sortable">
<j:choose>
<j:when test="${empty(app.pluginManager.plugins) and empty(app.pluginManager.failedPlugins)}">
Expand Down
35 changes: 0 additions & 35 deletions core/src/main/resources/hudson/PluginManager/sidepanel.groovy

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<!--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be helpful if the new side panel pattern would be exposed as tags in /lib/layout so that plugins can reuse this concept. Otherwise we will get a lot of copy and paste code when others try to implement that pattern in plugins.

Copy link
Contributor Author

@janfaracik janfaracik Jul 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely 👍 If it's alright with you though I'd like to wait a bit to have a more concrete design across Jenkins before building a new component/tag for these new sorts of sidebar behaviours as they may change/be tweaked still depending on future PRs.

For the most part though replicating this shouldn't be too hard with the <l:task ... /> component.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this is a good idea. As @daniel-beck noted as well:

This is the second PR introducing a title in the sidepanel (after #6485) without even making a note of that. Is that documented and discussed somewhere? How are we going to handle the problem of pages without a sidepanel, won't it be weird when some pages have a title above the main panel, while others have it in the sidepanel?

I think we should start very soon with creating and documenting a control for that (and maybe opening a discussion about the new concept can and should be used in plugins). Or when do you expect that others will pick up those changes for their plugins? Then we should at least notify everybody that this concept is not yet mature and should not be applied in plugins.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell #7051 is the codification of this design pattern. Am I correct in understanding that after #7051 is merged, this PR and the Configure Project redesign will be refactored to use the new codified design pattern, at which point this feedback will be completely addressed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The configure project is updated as part of #7051. The plugin manager on the other hand has been changed (I should probably update the opening post) to get sign off from Daniel so the title of the page is now above the search bar. This could change in the future in which case I'll definitely adopt the way #7051 does it 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't really follow the explanation regarding the relationship between this PR and #7051, but I trust that we are making progress. My primary concern is that the design pattern codified in #7051 is consistently followed by all implementations, including the configure project page and the plugin manager page (this PR) as well as any future pages that are converted to this pattern.

The order of operations seems like it is up for debate, but my main concern is just that we reach the desired end state and not that we take any particular order of operations to get there. I would expect that it would be desirable to fully flesh out the API in #7051 before adding new implementations as in this PR, because otherwise those implementations would then need to be adjusted later to conform to the final API. But I could be wrong about this. Either way, I think we should get the API finalized and all implementations conforming before we convert more pages to this paradigm.

The MIT License

Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
Copyright (c) 2022, Jenkins contributors

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand All @@ -23,17 +23,19 @@ THE SOFTWARE.
-->

<!--
List of available new plugins
Sidepanel component for Plugins Manager
-->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<st:documentation>
<st:attribute name="page"/>
</st:documentation>
<l:tabBar>
<l:tab name="${%Updates}" active="${page=='updates'}" href="." />
<l:tab name="${%Available}" active="${page=='available'}" href="./available" />
<l:tab name="${%Installed}" active="${page=='installed'}" href="./installed" />
<l:tab name="${%Advanced}" active="${page=='advanced'}" href="./advanced" />
</l:tabBar>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt">
<l:side-panel>
<l:tasks>
<l:task href="${rootURL}/manage/pluginManager/" icon="symbol-download" title="${%Updates}"/>
<l:task href="${rootURL}/manage/pluginManager/available" icon="symbol-shopping-bag" title="${%Available plugins}"/>
<l:task href="${rootURL}/manage/pluginManager/installed" icon="symbol-plugins" title="${%Installed plugins}"/>
<l:task href="${rootURL}/manage/pluginManager/advanced" icon="symbol-settings" title="${%Advanced settings}"/>
<j:if test="${!app.updateCenter.jobs.isEmpty()}">
<l:task href="${rootURL}/manage/pluginManager/updates/" icon="symbol-list" title="${%Download progress}"/>
</j:if>
</l:tasks>
</l:side-panel>
</j:jelly>

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading