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

Add height restriction option for EuiImage #1554

Closed
flash1293 opened this issue Feb 12, 2019 · 10 comments · Fixed by #3012
Closed

Add height restriction option for EuiImage #1554

flash1293 opened this issue Feb 12, 2019 · 10 comments · Fixed by #3012

Comments

@flash1293
Copy link
Contributor

The EuiImage component has the size parameter to set the width of an image, but it isn't possible to controll the height (or maximum height) which may come in handy if user supplied images with unkown aspect ratio are displayed using this component (e.g. elastic/kibana#30654)

Open for discussion how exactly this should look like.

@snide
Copy link
Contributor

snide commented Sep 12, 2019

Another nudge to @miukimiu since she's in this code already with #2287

Might be an easy prop to add.

@nik72619c
Copy link

Anyone working on this ?

@miukimiu
Copy link
Contributor

Hi @nik72619c,

I must have missed this while working on #2287.

Not sure if @andreadelrio is going to work on this. I'm seeing her assigned to this task.

But do you want to help?

@nik72619c
Copy link

yeah sure !

@nik72619c
Copy link

Just wanted some clarity on the fact that as there exists a class for setting the full width like euiImage--fullWidth, do we aspire to have the same with height ?

@snide
Copy link
Contributor

snide commented Feb 21, 2020

I think by default the height would adjust based upon the width and a "full height" isn't needed. Likely we just need a "max height" that would restrict the width if needed. In all cases the images original ratio should be maintained.

@nik72619c
Copy link

how much should be the "max height" ?

@snide
Copy link
Contributor

snide commented Feb 21, 2020

Talking about this with @cchaos we decided it likely makes sense that the size prop essentially means "width or height should be set to this, whichever is larger" in any cases when it is not set to original or fullWidth.

It would also be nice if the size prop also accepted a numeric value. You can see examples of how to accept a numeric value or enum in several of our other components. Off hand I know EuiPage's restrictWidth prop does something similar.

@jayanthjj
Copy link

Is anyone working on this issue?

@chandlerprall
Copy link
Contributor

Removing @andreadelrio from the assignee.

As the last comment was 5 days ago, I'm going to keep this un-assigned until the next time someone asks for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants