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

UI renovation #328

Merged
merged 7 commits into from
Feb 8, 2024
Merged

UI renovation #328

merged 7 commits into from
Feb 8, 2024

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Jan 9, 2024

  • remove all style attributes and replace with classes
  • use only standard jenkins variables and color so everything looks good with dark theme as well
  • do all styling via an explicit css file
  • use jenkins-table where appropriate
  • use a grid layout instead of tables to arrange the portlets
  • wrap the portlet in a div instead of a table
  • use new weather icons for job statistics
  • use ionicons for portlet collapse, expand, fullscreen
  • sort jobs by full name for job grid

fixes: #326
fixes: #294
fixes: #254
fixes: #202
fixes: #233
fixes: #230

Screenshots image image image

normal theme:
image

Testing done

Interactive testing

Submitter checklist

@mawinter69 mawinter69 force-pushed the ui-renovation branch 3 times, most recently from f86aec4 to 561f5d6 Compare January 10, 2024 00:40
- remove all style attributes and replace with classes
- use only standard jenkins variables and color so everything looks good
  with dark theme as well
- do all styling via an explicit css file
- use jenkins-table where appropriate
- use a grid layout instead of tables to arrange the portlets
- wrap the portlet in a div instead of a table
- use new weather icons for job statistics
- sort jobs by full name for job grid
@mawinter69 mawinter69 marked this pull request as ready for review January 10, 2024 01:11
@mawinter69 mawinter69 requested a review from a team as a code owner January 10, 2024 01:11
Copy link
Contributor

@TobiX TobiX left a comment

Choose a reason for hiding this comment

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

Looks awesome so far. Still need to test myself and would like to get some feedback from downstream (do they still exist?) plugin authors...

@TobiX
Copy link
Contributor

TobiX commented Jan 11, 2024

Pinging downstream plugin developers for tests: @tyanai @dalvizu @centic9 @JayDVector @uhafner

@uhafner
Copy link
Member

uhafner commented Jan 11, 2024

On my screen the widgets are too wide:

Bildschirmfoto 2024-01-11 um 19 16 44

@uhafner
Copy link
Member

uhafner commented Jan 11, 2024

It somehow looks strange that the new title has rounded corners and the table as well:

Bildschirmfoto 2024-01-11 um 19 18 31

I prefer integrated styles in the form similar to:

Bildschirmfoto 2024-01-11 um 19 22 17

@mawinter69
Copy link
Contributor Author

mawinter69 commented Jan 11, 2024

How about this?
image
The problem is that a jenkins-table defines a margin at the bottom
Also the inner table still has rounded corners (default jenkins style)

@mawinter69
Copy link
Contributor Author

On my screen the widgets are too wide:

Bildschirmfoto 2024-01-11 um 19 16 44

That behaviour can also be seen on the old dashboard
image

this avoids that the portlets start overlapping when the screen width
becomes too small and the tables can't be shrinked any further

remove some wrong classes
@uhafner
Copy link
Member

uhafner commented Jan 11, 2024

How about this?

This looks better, yes.

The problem is that a jenkins-table defines a margin at the bottom Also the inner table still has rounded corners (default jenkins style)

Why don't you create a new class that does work inside of a widget: this class can be based on the properties of the jenkins-table? I think it would look even better if there is no gap at the bottom (and no additional corners).

Maybe it would also make sense to get in touch with @timja and @janfaracik as well as they might plan something similar for the main pages (without an extra plugin). Maybe they have some styles for these widgets already.

- border around the content
- dedicated jelly for tables with tweaked styling
- move implementation guide to separate file
@mawinter69
Copy link
Contributor Author

Updated look:
image
Jenkins style adjusted so upper corners of the table are no longer rounded, margin a bottom is removed.

@uhafner
Copy link
Member

uhafner commented Jan 12, 2024

Jenkins style adjusted so upper corners of the table are no longer rounded, margin a bottom is removed.

Yes, that looks good now!

I still would increase the header height but this is a matter of taste. Then it would better match:

Bildschirmfoto 2024-01-12 um 08 35 40

@uhafner
Copy link
Member

uhafner commented Jan 12, 2024

On my screen the widgets are too wide:

That behaviour can also be seen on the old dashboard

I see, then this is another regression that has been introduced by the UI changes in core

@TobiX
Copy link
Contributor

TobiX commented Jan 12, 2024

On my screen the widgets are too wide

Interesting, this is something I noticed in the "Next Executions" Widget also (if browser window is between 900px & 1170px wide): jenkinsci/next-executions-plugin#78

@uhafner
Copy link
Member

uhafner commented Jan 15, 2024

Before you are merging this we need to make a plan how to update the affected plugins.

@mawinter69
Copy link
Contributor Author

Before you are merging this we need to make a plan how to update the affected plugins.

plugins implementing the portlet are not broken with this change. It's just that when they use inner tables it becomes more obvious that they use the old styling. Some plugins show images (graphs), that should work without issues.

@uhafner
Copy link
Member

uhafner commented Jan 15, 2024

UI tests are broken in my plugin now. I am not sure if others have UI tests though...

@TobiX
Copy link
Contributor

TobiX commented Feb 8, 2024

I would really like to merge this, but also don't want to break @uhafner's UI tests 😉 - Any progress on that front?

(I assume this is fine with other plugins, since we haven't received any feedback from their authors so far...)

@uhafner
Copy link
Member

uhafner commented Feb 8, 2024

I am currently preparing a bigger release that breaks UI tests as well. So feel free to merge, I will take care of it when the changes will be visible in the BOM.

@TobiX TobiX merged commit a74654f into jenkinsci:main Feb 8, 2024
16 checks passed
@uhafner
Copy link
Member

uhafner commented Feb 22, 2024

UI tests are broken in my plugin now. I am not sure if others have UI tests though...

This PR probably also caused all the failures in https://ci.jenkins.io/job/Core/job/acceptance-test-harness/job/master/lastCompletedBuild/testReport/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment