-
Notifications
You must be signed in to change notification settings - Fork 0
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
SFD-124: Limit the size/shape that the diagram can be #78
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks to work fine everywhere, the 0.7 feels a little arbitrary so maybe we just put it into a constant for some explanation?
As for aspect ratio, I'm not sure that there's anything that deals with it automatically, would just have to calculate from the width ie.
const MIN_RATIO = 9 / 16;
const MAX_RATIO = 16 / 9;
...
const minHeight = MIN_RATIO * innerWidth;
const maxHeight = MAX_RATIO * innerWidth;
Or something like that.
09d7241
to
8a443e0
Compare
b58be64
to
d42600f
Compare
SFD-124 Jira ticket
Caps the chart height based on screen height to prevent issues with the chart becoming stretched when the component is displayed in a tall iFrame.
In iFrame before fix:
In iFrame after fix:
I've capped based on screen height rather than aspect ratio for the time being, mostly due to the screen height cap seeming to work ok and being unfamiliar with using aspect ratio to set height and how it would work in various contexts. I think it looks ok on mobile views (checked with the Chrome device toggle) and laptop but we might need to revisit and use aspect ratio if we aren't getting the desired effect in some settings.