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

Revamp Design Library v3 #359

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Dec 15, 2024

This MR revamps the Design Library, refining every page for consistency and ensuring documentation is consistent and current.

The Design Library has been active for a couple of years now and has grown a fair amount, a result of this though is that its design has become a little unwieldy, different pages approach things in different ways, and this has made it a little hard to read and maintain. We also still have pages from the initial ui-samples version, some pages were left as-is, these felt out of place and didn't necessarily reflect the current state of Jenkins.

Pictures below

What's changed

  • Every page has been refreshed for consistency
  • Samples are now grouped into 'Components' or 'Patterns'
  • Pages now feature a consistent description at the top
  • Content and design has been cleaned up across the board
  • Inline tabs for switching languages for samples (e.g. Jelly and Java)
  • New 'Edit this page' button added
  • Respects prefers contrast and prefers reduced motion
  • Updated cards guidance
  • Pages can now declare what LTS they're available in

Fixes #352
Fixes #353
Fixes #328

@janfaracik janfaracik requested a review from a team as a code owner December 15, 2024 14:19
m.add(String.format("State %s in %s", s, country), country + ':' + s);
for (String s : asList("A", "B", "C")) {
m.add(String.format("State %s", s), s);
}

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If DescriptorImpl#doFillStateItems connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
m.add(String.format("State %s in %s", s, country), country + ':' + s);
for (String s : asList("A", "B", "C")) {
m.add(String.format("State %s", s), s);
}

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing permission check Warning

Potential missing permission check in DescriptorImpl#doFillStateItems
m.add(String.format("City %s in %s %s", s, state, country), state + ':' + s);
for (String s : asList("X", "Y", "Z")) {
m.add(String.format("City %s in %s", s, state), state + ':' + s);
}

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If DescriptorImpl#doFillCityItems connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
m.add(String.format("City %s in %s %s", s, state, country), state + ':' + s);
for (String s : asList("X", "Y", "Z")) {
m.add(String.format("City %s in %s", s, state), state + ':' + s);
}

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing permission check Warning

Potential missing permission check in DescriptorImpl#doFillCityItems
AutoCompletionCandidates c = new AutoCompletionCandidates();
for (String state : STATES)
if (state.toLowerCase().startsWith(value.toLowerCase())) {
c.add(state);

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If DescriptorImpl#doAutoCompleteState connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
AutoCompletionCandidates c = new AutoCompletionCandidates();
for (String state : STATES)
if (state.toLowerCase().startsWith(value.toLowerCase())) {
c.add(state);

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing permission check Warning

Potential missing permission check in DescriptorImpl#doAutoCompleteState
@janfaracik
Copy link
Contributor Author

janfaracik commented Dec 15, 2024

Some screenshots

image

Buttons

Cards

Colors

Inputs

JavaScript Proxy

Select

Would recommend giving the plugin a spin to see all changes.

@janfaracik
Copy link
Contributor Author

janfaracik commented Dec 15, 2024

All changes

Components

Home

  • Design refreshed slightly, now respects prefers reduced motion
  • Icons updated
  • Option to go to the next page added

App bars

  • Titles added
  • New icon

Banner

  • Split out into different sections
  • Notice removed

Buttons

  • Content updated

Cards

  • Mild design changes
  • Content added for how/when to use cards

Checkboxes

  • Design updated

Dialogs

  • Split out into different sections

Dropdowns

  • Mild design changes

File

  • Split out into different sections

Inputs

  • Content updated
  • Added Search
  • Autocompletes removed, moved to Select page

Notifications

  • Split out into different sections

Progress

  • Split out into different sections

Radios

  • Content updated
  • Jelly/Java combo moved into tabs

Repeatable list

  • Updated content, no longer 'Hetero list'
  • Full page refresh
    • Example is now at the top, rather than having to click 'Configure me!'

Select

  • Full page refresh
  • Simplified examples
  • Added autocomplete

Symbols

  • Mild design changes

Table

  • Table now inlined

Toggle switch

  • Split out into different sections

Tooltips

  • Mild design changes

Patterns

Colors

  • Full page refresh
    • Still not completely happy with it, but there's more space for content now

JavaScript Proxy

  • Full page refresh
    • Big sample at the top previewing what it does

Layouts

  • Mild design changes

Links

  • Mild design changes

Spacing

  • Simplified example

Stylesheets

  • Mild design changes

Validation

  • Mild design changes
    • Still not completely happy with it

</div>
<s:code language="xml" file="index.jelly" />
</div>
<div class="jenkins-alert jenkins-alert-warning">
Copy link
Member

Choose a reason for hiding this comment

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

do we still need this warning? its not that old at this point?

do.1=Keep your card glanceable, let the user expand your card for more information
do.2=Make your cards scalable to different screen sizes
do.3=Do use hierarchy and symbols to add visual context
dont.1=Don''t add unnecessary visual elements that clutter the card
dont.2=Don''t have too many actions on one card
creating-a-card.title=Creating a card
creating-a-card.description=Cards are designed to automatically expand and fill the available space within their container, making them flexible and easy to use in different layouts. \n\nSince Jenkins doesn''t currently have a built-in grid system, you''ll need to create your own if you want to arrange multiple cards in a grid-like structure. You can do this by utilizing Flexbox or Grid to manage the positioning and spacing of your cards within the parent container.
Copy link
Member

Choose a reason for hiding this comment

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

text="${%Action 4}" />
<dd:separator />
<dd:submenu icon="symbol-star-outline plugin-ionicons-api" text="${%Submenu}">
<s:layout>
Copy link
Member

Choose a reason for hiding this comment

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

general feedback but pre-existing issue. A dropdown to me is generally what's on the select page, this page is a (sub)type of button

<s:section hideBorder="true">
<s:group>
<s:preview fullWidth="true">
<f:property field="config" />
Copy link
Member

Choose a reason for hiding this comment

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

Can't seem to use this on a 13" mac, it overflows under the example content:
image

@@ -43,7 +44,7 @@
import org.kohsuke.stapler.StaplerRequest;

@Extension
public final class HeteroList extends UISample {
public class HeteroList extends UISample {
Copy link
Member

Choose a reason for hiding this comment

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

Rename to RepeatableList to make the class easier to find?

return c;
}

public ComboBoxModel doFillStateItems() {
Copy link
Member

Choose a reason for hiding this comment

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

this example seems to be missing?

usage.1=You can use <code>dropdownDescriptorSelector</code> to create a fully data-bound form and the option to include a \
config page for the <code>descriptor</code> by creating a file called <code>config.jelly</code>.
usage.2=If you want a simpler <code>select</code> to be rendered you can use the <code>f:select</code> \
Jelly tag and create a <code>doFillFieldNameItems</code> method in the <code>descriptor</code>.
dynamic=Dynamic select
dynamic.description=Updates the contents of a "select" control dynamically based on selections of other controls.
searchable=Searchable select
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is actually a searchable select, its an auto complete, but I don't think you are forced to use the text from this (would need to check)

<form action="submit" method="post">
<s:code language="xml" file="sample.jelly" />
<f:dropdownDescriptorSelector field="fruit" title="Fruits" descriptors="${it.fruitDescriptors}" />
<s:section title="Advanced select" description="${%usage.1}">
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 ideal if this example also showed the Java code as well

blurb=<p class="jdl-paragraph">Jenkins consists of a large and complex graph of domain objects (<code>ModelObject</code>), where each node in the graph is a web page and edges are hyperlinks. \
To help users navigate quickly on this graph, Jenkins provides a mechanism to attach context menus to model objects, \
which can be used to hop multiple edges without going through individual hyperlinks.

define.title=Defining context menu

blurb.define=<p class="jdl-paragraph">To define a context menu on <code>ModelObject</code>, implement <a href="https://javadoc.jenkins.io/byShortName/ModelObjectWithContextMenu" target="_blank"><code>ModelObjectWithContextMenu</code></a>.
blurb.define=To define a context menu on <code>ModelObject</code>, implement <a href="https://javadoc.jenkins.io/byShortName/ModelObjectWithContextMenu" target="_blank"><code>ModelObjectWithContextMenu</code></a>.
Copy link
Member

Choose a reason for hiding this comment

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

Rendering of this looks quite broken:
image

Copy link
Member

Choose a reason for hiding this comment

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

Title on the page is also:
image


<h3>${%examples}</h3>

<div class="jdl-component-sample">
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to have more examples on this page, its too texty to me really, instead of saying jenkins-!-margin-* it would be good to have examples

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

Generally looks really good. I've checked every page and compared to what it was before. Few things though I think could do with fixing before merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants