-
-
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
Allow for more flexible layouts in layout.jelly
#8794
base: master
Are you sure you want to change the base?
Conversation
@@ -27,8 +27,7 @@ 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 permissions="${app.MANAGE_AND_SYSTEM_READ}" title="${%System}" type="one-column"> |
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.
One issue with the current approach is that there can be a mismatch between the page type and it's contents - on the Configure
page it includes a sidebar however it isn't displayed due to the type being set to one-column
. The inverse could be true as well where the page type is two-column
but a side panel hasn't been provided.
@@ -34,16 +34,14 @@ THE SOFTWARE. | |||
Generates a row containing the page title and an optional set of controls | |||
</st:documentation> | |||
|
|||
<j:if test="${mode=='main-panel' or mode=='side-panel'}"> |
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.
This if
condition wasn't needed so I've removed it.
@@ -58,7 +58,7 @@ THE SOFTWARE. | |||
(The permissions will be checked against the "it" object.) | |||
</st:attribute> | |||
<st:attribute name="type" use="optional"> | |||
Available values: two-column (by default), one-column (full-width size) or full-screen (since 2.53). |
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 type
attribute now only works if it's set to full-screen
.
@@ -3,7 +3,6 @@ | |||
.jenkins-section { | |||
border-top: 2px solid var(--panel-border-color); | |||
padding: var(--section-padding) 0 0 0; | |||
max-width: 1800px; |
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.
Simplified the styling, let's just fill the available width and let the parent container control how wide that can be.
<j:set var="mode" value="side-panel" /> | ||
<d:invokeBody /> |
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.
Breaks all context menus, see ModelObjectWithContextMenu.ContextMenu#from
.
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.
Thanks! Fixed it and added a comment to side-panel.jelly
This PR looks a lot like
would apply (on a UI level), but I don't see consumers for this change? |
Please take a moment and address the merge conflicts of your pull request. Thanks! |
I've added some guidance on using the new layout system in the Design Library - jenkinsci/design-library-plugin#315 - intending to update the UI of the plugin itself to use a multi column layout, however I haven't had the time recently. |
Please take a moment and address the merge conflicts of your pull request. Thanks! |
Please take a moment and address the merge conflicts of your pull request. Thanks! |
possible to update the UI to use it? should really have a consumer before shipping something like this |
Will do 👍 |
Please take a moment and address the merge conflicts of your pull request. Thanks! |
Updated the Design Library with an additional side-panel which gives an overview of the page jenkinsci/design-library-plugin#315 |
Please take a moment and address the merge conflicts of your pull request. Thanks! |
I don't think the UI demo plugin qualifies as a consumer that would validate that this change does anything useful. |
Please take a moment and address the merge conflicts of your pull request. Thanks! |
Background
Currently in Jenkins you're limited to three layout types -
two-column
,one-column
andfull-screen
. This PR reworkslayout.jelly
to allow for more layout types, for example a three column layout or a layout with the side panel on the right.This gives developers more flexibility in how they approach the design of their pages and also simplifies how
layout.jelly
and it's related stylings work.Testing done
I'll raise a PR to fix thatPR has been raised Add 'On this page' side-panel and add supporting docs for 'Allow for more flexible layouts in layout.jelly #8794' design-library-plugin#315Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Desired reviewers
@jenkinsci/sig-ux
Before the changes are marked as
ready-for-merge
:Maintainer checklist