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

width and height fails for Charts #5126

Closed
Danieleeee opened this issue Feb 12, 2018 · 14 comments
Closed

width and height fails for Charts #5126

Danieleeee opened this issue Feb 12, 2018 · 14 comments
Assignees
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@Danieleeee
Copy link

Danieleeee commented Feb 12, 2018

I'm submitting a ...

[X] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primeng/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=35

Current behavior

p-chart attribute height = "50%" don't work in PRIME NG 5.2.0, work in PRIME NG 5.0.2

What is the motivation / use case for changing the behavior?

Please tell us about your environment:

  • Angular version: 5.2.X
  • PrimeNG version: 5.2.X
  • Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]
  • Language: [all | TypeScript X.X | ES6/7 | ES5]

  • Node (for AoT issues): node --version =

@sagar-shirsath
Copy link

Hello Primeng team, we are using primeng in one of our project, will you please accept merge request raised by Sujay.

@dhniels
Copy link

dhniels commented Feb 22, 2018

im having this bug as well. assigning a height of 85 to a p-chart, the height generated in the dom on the canvas is 150.

thank you @sagar-shirsath for the PR, hopefully PrimeNG team accepts it and pushes out a fix ASAP.

update: i tested adding a width rule to p-chart and it doesnt work either.

@cagataycivici
Copy link
Member

cagataycivici commented Mar 4, 2018

According to the chart.js, width and height to the canvas is wrong;

http://www.chartjs.org/docs/latest/general/responsive.html

So we need to apply them maybe if chart is not responsive?

@cagataycivici cagataycivici changed the title p-chart, height attribute don't work width and height fails for Charts Mar 4, 2018
@cagataycivici cagataycivici self-assigned this Mar 4, 2018
@cagataycivici cagataycivici added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Mar 4, 2018
@cagataycivici cagataycivici added this to the 5.2.1 milestone Mar 4, 2018
@cagataycivici
Copy link
Member

cagataycivici commented Mar 4, 2018

Fixed according to the suggestions of charts.js docs, the PR might have caused blur plots for responsive charts. Added docs as well.

Responsive
Charts are responsive by default and vw-vh units should be used when customizing the width and height of the chart.

<p-chart type="line" [data]="data" width="40w" height="80w"></p-chart>

If the chart is not responsive, other units should be preferred.

<p-chart type="line" [data]="data" width="400px" height="400px" [responsive]="false"></p-chart>

@dhniels
Copy link

dhniels commented Mar 7, 2018

@cagataycivici what about if you wanted to do what i was doing in a couple instances, where it was responsive but had a set height. for example, i have a chart inside of a box on the page that i need to stay a fixed height but is allowed to grow horizontally with the viewport.

im on 5.2.1 and im still having this issue. I have a height of 90px on a chart and it wont respect that unless i make the chart non-responsive. before it be responsive in terms of width but the height would be fixed at 90px.

im a little confused by the use of vw and vh units, it seems like in most cases you would want to size the chart to a container instead of the viewport. wouldnt percentage values relative to its container be better?

@dhniels
Copy link

dhniels commented Mar 7, 2018

this issue appears to still be present on 5.2.1. ive tried setting width and height and it wont honor either value unless i set responsive to false. in the meantime ive removed the height, width, and responsive attributes and applied the following css: ::ng-deep .chart-container canvas {height: 90px !important;}

also, on your first example, you're saying to use vw and vh units but on the actual attribute you're setting them to 40w and 80w, is that a typo? Did you mean to type 40vw and 80vh?

also you say "Charts are responsive by default" but on the p-chart documentation page, it says the default value is false.

screen shot 2018-03-07 at 4 51 50 pm

@Danieleeee
Copy link
Author

Issue still present in 5.2.3

@JacobSiegle
Copy link
Contributor

JacobSiegle commented Apr 12, 2018

5.2.0 is the last working version for a responsive chart with fixed height. Can this issue be opened again?

If height is specified or response mode is false can it be applied?
[style.height]="responsive && height == null ? null : height"

[attr.height]="responsive && height == null ? null : height"

@rich4756
Copy link

Issue still present in 5.2.4

@fernandocode
Copy link

@dhniels , I had the same problem and opened a new question here #5748 , I'm even thinking about sending a PR with the fix, meanwhile I'm using a custom component with my implementation instead of the p-chart.

If you want to check my solution and possibly suggest something.

@JacobSiegle
Copy link
Contributor

Put in #5858 to hopefully fix the issue with sizing in responsive mode and the doc errors.

@cagataycivici
Copy link
Member

PR merged.

@Swathi-Gangula
Copy link

issue is still present in primeng 5.2.7 version

@JacobSiegle
Copy link
Contributor

The fix was merged into the ng 6 branches, so you need a newer version of PrimeNG or manually pull and include the fix yourself.

geyuqiu pushed a commit to geyuqiu/SfoGateAssignmentProblem that referenced this issue Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

8 participants