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

PAYARA-4097 Fixed Grid Layout, Dynamic Settings #4225

Merged
merged 3 commits into from
Sep 20, 2019

Conversation

jbee
Copy link
Contributor

@jbee jbee commented Sep 18, 2019

Summary

Pure JS/HTML/CSS update for MC webapp that does:

  • fixed grid layout algorithm
  • made settings dynamic or responsive so they reflect the actual chart selected and its current values - for this the settings are now purely generated by JS
  • organised and cleaned the JS code (main separation into model view and chart specific code all under the global object MonitoringConsole)

Reviewer Notes

Remember that monitoring-console.js is merged from the other JS files and does not need to be reviewed.

@jbee jbee added this to the 5.194 milestone Sep 18, 2019
@jbee jbee self-assigned this Sep 18, 2019
@jbee
Copy link
Contributor Author

jbee commented Sep 18, 2019

jenkins test please

@jbee
Copy link
Contributor Author

jbee commented Sep 18, 2019

jenkins test please

Copy link

@AlanRoth AlanRoth left a comment

Choose a reason for hiding this comment

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

Looking good! However, I'm not sure if you implemented it but I was not able to remove any charts that I added, and are you able to add charts to monitor instances other than server?

@jbee
Copy link
Contributor Author

jbee commented Sep 19, 2019

@AlanRoth Removing charts is implemented but I discovered that I broke it recently (and fixed it later). Other instances can be monitored. Just start one and if it has a series with non zero value it should occur in the chart and also in the list of instances. I might also have broken something there recently since the list of instances actually has an effect of the shown data. Have to check.

@jbee
Copy link
Contributor Author

jbee commented Sep 19, 2019

@AlanRoth Other instances still works. Just create them and the core tab should start showing multiple lines.

@jbee
Copy link
Contributor Author

jbee commented Sep 19, 2019

@AlanRoth Since I already fixed the issue with chart removal in another branch I hope it is ok for you if we keep it in this one as it is not related to the feature of this PR and was caused by general code reorganisation I have to do every now and then in the JS code to keep it readable. Slowly I learn how to do that without it growing to big or confusing.

@jbee jbee merged commit 9c4d2c9 into payara:master Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants