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.

Sign up for GitHub

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

Jump to bottom

Adding width and height as parameters to image url #46917

Merged
merged 5 commits into from
Oct 7, 2019
Merged

Adding width and height as parameters to image url #46917

merged 5 commits into from
Oct 7, 2019

Conversation

majagrubic
Copy link
Contributor

@majagrubic majagrubic commented Sep 30, 2019 •
edited
Loading

Summary

Closes #10109.
This PR adds two new fields when setting an url of type image: width and height. These are then used to set the appropriate image size, in a way that the image is never scaled up, only down.
image_resize

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@majagrubic majagrubic added Feature:FieldFormatters Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 30, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@majagrubic majagrubic requested review from timroes and removed request for timroes September 30, 2019 14:03
timroes
timroes previously requested changes Sep 30, 2019
View reviewed changes
src/legacy/ui/public/field_editor/components/field_format_editor/editors/url/url.js Outdated Show resolved Hide resolved
src/legacy/ui/public/field_editor/components/field_format_editor/editors/url/url.test.js Outdated Show resolved Hide resolved
src/legacy/core_plugins/kibana/common/field_formats/types/url.js Outdated Show resolved Hide resolved
src/legacy/core_plugins/kibana/common/field_formats/types/url.js Outdated Show resolved Hide resolved
src/legacy/core_plugins/kibana/common/field_formats/types/url.js Outdated Show resolved Hide resolved
src/legacy/ui/public/field_editor/components/field_format_editor/editors/url/url.js Outdated Show resolved Hide resolved
src/legacy/ui/public/field_editor/components/field_format_editor/editors/url/url.js Show resolved Hide resolved
@timroes timroes requested a review from flash1293 September 30, 2019 15:07
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese
Copy link
Contributor

nreese commented Sep 30, 2019

Thanks for adding support for width and height. Would it be possible to add a quick sentence in the docs https://www.elastic.co/guide/en/kibana/current/field-formatters-string.html about how width and height are in pixels and how they work.

@timroes timroes self-requested a review October 1, 2019 12:02
flash1293
flash1293 approved these changes Oct 2, 2019
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested and works, LGTM 👍 Added one small nitpick, but nothing to block merging

src/legacy/ui/public/field_editor/components/field_format_editor/editors/url/url.js Outdated
Comment on lines 144 to 145
isInvalid={!!error}
error={error}
Copy link
Contributor

Choose a reason for hiding this comment

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

I might miss something, but how do I get into the error state? Might be a left-over of a former refactoring

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Widht/height changes won't throw errors for now. I was thinking we should add some limitations on max/min values and then display errors accordingly. Not sure if it is necessary though and happy to remove these for now.

src/legacy/core_plugins/kibana/common/field_formats/types/__tests__/url.js Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@timroes timroes dismissed their stale review October 4, 2019 07:34

Outdated

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@majagrubic majagrubic merged commit 9413939 into elastic:master Oct 7, 2019
@nreese nreese mentioned this pull request Oct 7, 2019
Closed
@majagrubic majagrubic mentioned this pull request Nov 12, 2019
Closed
majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Nov 12, 2019
Adding width and height as parameters to image url (elastic#46917)
9a9ae40 
* Adding width and height as parameters to image url

* Addressing PR comments

* Removing unnecessary error message
@majagrubic majagrubic mentioned this pull request Nov 12, 2019
Closed
majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Nov 12, 2019
Adding width and height as parameters to image url (elastic#46917)
07a3d8a 
* Adding width and height as parameters to image url

* Addressing PR comments

* Removing unnecessary error message
@bcamper bcamper mentioned this pull request Nov 12, 2019
Closed
@majagrubic majagrubic mentioned this pull request Nov 12, 2019
Merged
majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Nov 12, 2019
Adding width and height as parameters to image url (elastic#46917)
b5a5311 
* Adding width and height as parameters to image url

* Addressing PR comments

* Removing unnecessary error message
majagrubic pushed a commit that referenced this pull request Nov 13, 2019
Adding width and height as parameters to image url (#46917) (#50406)
86f0c17 
* Adding width and height as parameters to image url

* Addressing PR comments

* Removing unnecessary error message
@majagrubic majagrubic mentioned this pull request Dec 7, 2019
Closed
majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Dec 7, 2019
Adding width and height as parameters to image url (elastic#46917)
7f4aac1 
* Adding width and height as parameters to image url

* Addressing PR comments

* Removing unnecessary error message
@majagrubic majagrubic mentioned this pull request Dec 7, 2019
Merged
majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Dec 7, 2019
Adding width and height as parameters to image url (elastic#46917)
0bffa99 
* Adding width and height as parameters to image url

* Addressing PR comments

* Removing unnecessary error message
@nreese
Copy link
Contributor

nreese commented Dec 7, 2019

7.x #50406
7.5.1 #52457

majagrubic pushed a commit that referenced this pull request Dec 7, 2019
Adding width and height as parameters to image url (#46917) (#52457)
1d4e7ba 
* Adding width and height as parameters to image url

* Addressing PR comments

* Removing unnecessary error message
@majagrubic majagrubic deleted the maja/fix-10109 branch December 9, 2019 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nreese nreese nreese left review comments

@flash1293 flash1293 flash1293 approved these changes

@timroes timroes Awaiting requested review from timroes

Assignees
No one assigned
Labels
Feature:FieldFormatters release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.5.1 v7.6.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add "size" parameter for "img" fields in index patterns
5 participants
@majagrubic @elasticmachine @nreese @timroes @flash1293