Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix for graph height declared in template #262

Merged
merged 1 commit into from
Aug 24, 2022
Merged

Conversation

dheule
Copy link
Contributor

@dheule dheule commented Feb 23, 2022

This shuld fix #234

@nilmerg
Copy link
Member

nilmerg commented Feb 23, 2022

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Feb 23, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: SFS sapadm.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Feb 28, 2022

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @dheule

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please contact us if you think this is the case.

  • If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.

@bobapple
Copy link
Member

bobapple commented Mar 1, 2022

@cla-bot check

@cla-bot cla-bot bot added the cla/signed label Mar 1, 2022
@nilmerg
Copy link
Member

nilmerg commented Mar 22, 2022

Doesn't work for me.
Screenshot from 2022-03-22 12-04-24

There seems to be something wrong with how the template params height and width are handled anyway. If you want to continue, please let me know, I will then describe the solution I think we need in the issue.

@dheule
Copy link
Contributor Author

dheule commented Apr 3, 2022

Doesn't work for me. Screenshot from 2022-03-22 12-04-24

There seems to be something wrong with how the template params height and width are handled anyway. If you want to continue, please let me know, I will then describe the solution I think we need in the issue.

Hi @nilmerg ,

I have only added the parameter processing from the height graph template to the render view, this is working.

But for Pie graphs, you also need a fixed width. I will look at this at the next few weeks, atm i have no time to implemnt this asap.

So please stay tuned for the next update. This pull request only helps for normal graphs with needs a bigger height.

Best regards,
Daniel

@dheule
Copy link
Contributor Author

dheule commented Jun 13, 2022

Now i have updated the fix for 1.2 and added the handling of width
@nilmerg : setting width and height to the same value should now fix your problem with the pie graphs.

Copy link
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

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

Yeah, looks correct now. No matter if I define only a width, or height, or both. 👍

@nilmerg nilmerg added this to the v1.2.2 milestone Aug 24, 2022
@nilmerg nilmerg added the bug Something isn't working label Aug 24, 2022
@nilmerg nilmerg merged commit 9e14387 into Icinga:master Aug 24, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working cla/signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Graph is not resized properly (height is capped)
3 participants