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.

Sign up for GitHub

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

Jump to bottom

Update values table CSS style and number formatting #47

Merged
merged 1 commit into from
May 15, 2018
Merged

Update values table CSS style and number formatting #47

merged 1 commit into from
May 15, 2018

Conversation

radekdoulik
Copy link
Contributor

@radekdoulik radekdoulik commented May 15, 2018 •
edited by vgaidarji
Loading

What has been done

  1. Added internal CSS section to the jelly file and added style the values table.
  2. Format the numbers in the table for improved readability

Screenshots

Before After
screen shot 2018-05-15 at 16 00 05 screen shot 2018-05-15 at 15 48 27

How to test

  1. View plot with values table

Checklist

  • Git commits follow best practices
  • Build passes in Jenkins
  • Appropriate tests or explanation to why this change has no tests
  • JIRA issue is well described (problem explanation, steps to reproduce, screenshots)
  • For dependency updates: links to external changelogs and, if possible, full diffs

@vgaidarji vgaidarji self-assigned this May 15, 2018
@vgaidarji
Copy link
Contributor

@radekdoulik thanks a lot, it looks really nice 👍. I'm going to test this out soon (today-tomorrow) and we can release the change right after that.
Could you please update the commit message in the meantime so that it follows the convention?

@radekdoulik
Copy link
Contributor Author

Updated the commit message. Hopefully it is OK now.

@vgaidarji
Copy link
Contributor

@radekdoulik well, it's still not 🙃It should be like Update values table CSS style and number formatting. I would really appreciate if you can adjust that, but if no - nevermind, it was a long way 🙂
I'm testing out the changes right now, should be able to merge/release it soon.

@radekdoulik
Update values table CSS style and number formatting
47b957b 
Added `<style>` section to the *jelly* file with CSS style for data
values table, nice borders and light gray header background.

Added `formatNumber` method to format the numbers in the table for
improved readability and used it in the *jelly* file.
@radekdoulik radekdoulik changed the title Style changes and number formatting Update values table CSS style and number formatting May 15, 2018
@radekdoulik
Copy link
Contributor Author

OK, the updated the commit message 47b957b

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request May 15, 2018
@jonathanpeppers
[timing] only output whole numbers to CSV file
370e6ed 
Context: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-android/plot/Build%20times/

The Jenkins plot plugin does not seem to be able to render decimal
numbers in the table displaying the CSV file.

Radek has sent a PR upstream to the plugin: jenkinsci/plot-plugin#47

In the meantime, we can fix this ourselves by only emitting whole
numbers to the CSV file. Since the build times are taking multiple
seconds and we are storing milliseconds in the CSV, this loss of
precision won't affect us much.
@vgaidarji
Copy link
Contributor

Here're my test results:

Before

Full table
screen shot 2018-05-15 at 12 06 49
Partially full table
screen shot 2018-05-15 at 12 06 59

After

Full table
screen shot 2018-05-15 at 12 59 28

Partially full table
screen shot 2018-05-15 at 12 59 41

And after the change table is more compact and more elements fit on screen.

Before After
screen shot 2018-05-15 at 13 08 31 screen shot 2018-05-15 at 13 01 51

@radekdoulik Thanks you so much for this improvement 💪

@vgaidarji vgaidarji merged commit 83e0c19 into jenkinsci:master May 15, 2018
@vgaidarji
Copy link
Contributor

vgaidarji commented May 15, 2018 •
edited
Loading

@radekdoulik fix was released as part of 2.0.5 🎉 (link to changelog).
New version will be available soon via update center (link to plot plugin artifacts in Jenkins artifacts repository).

@radekdoulik
Copy link
Contributor Author

Thank you for the quick release! 👍

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

@vgaidarji vgaidarji

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@radekdoulik @vgaidarji