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

Overhaul ui-samples as the Jenkins Design Library #14

Merged
merged 112 commits into from
Mar 29, 2022
Merged

Overhaul ui-samples as the Jenkins Design Library #14

merged 112 commits into from
Mar 29, 2022

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Mar 19, 2022

image

This is a WIP.

What's new

  • Overhauled design
  • New homepage
  • Simplified navigation makes it easy to switch between pages
  • Pages have been reorganised and rewritten for simplicity
  • New inline code samples

More screenshots

image

image

image


🎉 Major thanks to @timja for all the help and contributions


If you're wanting to try this, I really recommend using this branch https://github.com/janfaracik/jenkins/tree/scalable-icon-support-3-custom-symbols of Jenkins as it features the newer sidebar navigation and support for custom plugin symbols.


  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

  • In HeteroList#getConfigFile() we attempt to load "stuff.xml" from the root directory but never make any promises to create the file if it doesn't exist, therefore it errors if it actually doesn't exist.
  • Progress#factor() parses the input as int and likely never completes an operation while throwing a NumberFormatException if you enter a bigger number beyond the integer's max value.

Though, that are more logical concerns, the UI and its examples look really great 🎉

@timja
Copy link
Member

timja commented Mar 27, 2022

  • In HeteroList#getConfigFile() we attempt to load "stuff.xml" from the root directory but never make any promises to create the file if it doesn't exist, therefore it errors if it actually doesn't exist.

Do you mean if the root directory doesn't exist (not sure if that's possible)?

We check if the file exists here:
https://github.com/janfaracik/ui-samples-plugin/blob/08853e0dc7172ad880c43a244298f86819c2e279/src/main/java/io/jenkins/plugins/designlibrary/HeteroList.java#L57-L60

@NotMyFault
Copy link
Member

  • In HeteroList#getConfigFile() we attempt to load "stuff.xml" from the root directory but never make any promises to create the file if it doesn't exist, therefore it errors if it actually doesn't exist.

Do you mean if the root directory doesn't exist (not sure if that's possible)?

We check if the file exists here: https://github.com/janfaracik/ui-samples-plugin/blob/08853e0dc7172ad880c43a244298f86819c2e279/src/main/java/io/jenkins/plugins/designlibrary/HeteroList.java#L57-L60

Looks like I had a faulty reference from the ui samples -> design lib transition somewhere xstream picked up https://gist.githubusercontent.com/NotMyFault/0c919f268cf84e95a6b8ee061083393a/raw/18c622887c48a0e69c68d11eaf04abd8ba780f31/gistfile1.txt
A fresh setup of the core PR with an installation of this PR got a rid of it.

@timja
Copy link
Member

timja commented Mar 27, 2022

Looks like I had a faulty reference from the ui samples -> design lib transition somewhere xstream picked up

Makes sense, can probably refactor it to not do manual XML File handling.

Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

Looks fine to be shipped as (a first) overhaul, huge thanks to everyone contributing 🎉

Comment on lines +10 to +16
<button class="jenkins-button jenkins-button--primary">
<l:icon src="symbol-add" />
Primary control
</button>
<button class="jenkins-button">Secondary</button>
<button class="jenkins-button">Secondary</button>
<t:help href="https://www.jenkins.io" />
Copy link
Member

@timja timja Mar 28, 2022

Choose a reason for hiding this comment

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

the spacing looks not good on the buttons

image

is that just the branch I'm using (jenkinsci/jenkins#6395)

Copy link
Member

Choose a reason for hiding this comment

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

That is part of the latest weekly as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix for that should be in with the help button component merge 👍

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 my branch has the help button component update in it

document.querySelector('#button-share').style.display = "none"
}

document.querySelector('#button-share').addEventListener('click', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Looks like html unit breaks on this syntax, https://github.com/jenkinsci/ui-samples-plugin/pull/14/checks?check_run_id=5716218242

missing formal parameter

may be related to HtmlUnit/htmlunit#232

@timja timja enabled auto-merge (squash) March 29, 2022 08:04
@timja timja merged commit 57437b2 into jenkinsci:master Mar 29, 2022
froque added a commit to froque/jenkins that referenced this pull request Jan 28, 2024
ui-samples-plugin is suspended and has been replaced with design-library:

> Plugin distribution has been suspended, see https://groups.google.com/g/jenkinsci-dev/c/2vGn3t9gZ0Y for details.
https://plugins.jenkins.io/ui-samples-plugin/

jenkinsci/design-library-plugin#14
MarkEWaite pushed a commit to jenkinsci/jenkins that referenced this pull request Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants