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

[Enterprise Search] Added Thumbnails to Search UI #104199

Merged
merged 7 commits into from
Jul 14, 2021
Merged

[Enterprise Search] Added Thumbnails to Search UI #104199

merged 7 commits into from
Jul 14, 2021

Conversation

JasonStoltz
Copy link
Member

@JasonStoltz JasonStoltz commented Jul 1, 2021 •
edited
Loading

Summary

This PR adds thumbnail support to the Search UI page within App Search.

latest

Testing:

  • Make sure that the thumbnails work, both when generating a UI and when downloading a ZIP file.
  • Make sure that when the new Thumbnail dropdown is selected, that the correct portion of the image to the right of the form is highlighted

Release note

Adds a Thumbnail Image option to the Search UI page within App Search.

Checklist

@JasonStoltz JasonStoltz requested a review from a team July 1, 2021 19:51
@JasonStoltz JasonStoltz added release_note:enhancement auto-backport Deprecated - use backport:version if exact versions are needed labels Jul 1, 2021
@JasonStoltz
Copy link
Member Author

@elasticmachine merge upstream

@JasonStoltz
Copy link
Member Author

@elasticmachine merge upstream

@cee-chen
Copy link
Contributor

cee-chen commented Jul 9, 2021

@JasonStoltz since you've added the release_note:enhancement label, you'll need to add a ## Release note section. Check out the Kibana docs for more info:

https://www.elastic.co/guide/en/kibana/master/contributing.html#_create_the_release_notes_text

cee-chen
cee-chen reviewed Jul 9, 2021
View reviewed changes
Copy link
Contributor

@cee-chen cee-chen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, all super optional/minor comments. My only blocking comment is that you need a ## Release note header in your PR description if you want this to have a release note

x-pack/plugins/enterprise_search/public/applications/app_search/components/search_ui/types.ts Show resolved Hide resolved
x-pack/plugins/enterprise_search/public/applications/app_search/components/search_ui/i18n.ts Outdated Show resolved Hide resolved
...search/public/applications/app_search/components/search_ui/components/search_ui_graphic.scss Outdated Show resolved Hide resolved
...ise_search/public/applications/app_search/components/search_ui/components/search_ui_form.tsx
<EuiSelect
disabled={dataLoading}
options={optionFields}
value={selectedThumbnailOption && selectedThumbnailOption.value}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check, we don't anticipate adding a default value for this the same way we do for title and url right? Might be worth checking with the Crawler folks perhaps to see if they scrape a page thumbnail image?...

Copy link
Member Author

Choose a reason for hiding this comment

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

I can check with them

Copy link
Member Author

Choose a reason for hiding this comment

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

Will add defaulting to a future PR.

@JasonStoltz
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine kibanamachine requested a review from a team as a code owner July 12, 2021 14:48
JasonStoltz and others added 2 commits July 12, 2021 10:48
@JasonStoltz
Update x-pack/plugins/enterprise_search/public/applications/app_searc…
4152070 
…h/components/search_ui/i18n.ts

Co-authored-by: Constance <[email protected]>
@JasonStoltz
Update x-pack/plugins/enterprise_search/public/applications/app_searc…
73f35ec 
…h/components/search_ui/components/search_ui_graphic.scss

Co-authored-by: Constance <[email protected]>
@cee-chen
Copy link
Contributor

@JasonStoltz Quick check on the release notes section in your PR - I'm still not seeing that currently on page refresh, did that get added?

@JasonStoltz JasonStoltz changed the title [App Search] Added Thumbnails to Search UI [Enterprise Search] Added Thumbnails to Search UI Jul 12, 2021
@JasonStoltz
Copy link
Member Author

@constancecchen

@JasonStoltz Quick check on the release notes section in your PR - I'm still not seeing that currently on page refresh, did that get added?

Done, thank you for the reminder.

cee-chen
cee-chen approved these changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

🎉

@JasonStoltz
Copy link
Member Author

@daveyholler I need a code owner review, do you mind?

daveyholler
daveyholler approved these changes Jul 13, 2021
View reviewed changes
Copy link
Contributor

@daveyholler daveyholler left a comment

Choose a reason for hiding this comment

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

LGTM

@JasonStoltz
Copy link
Member Author

@elasticmachine merge upstream

@JasonStoltz JasonStoltz enabled auto-merge (squash) July 14, 2021 17:31
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.1MB 2.1MB +2.7KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@JasonStoltz JasonStoltz merged commit 02cd508 into elastic:master Jul 14, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 14, 2021
@JasonStoltz @kibanamachine
[Enterprise Search] Added Thumbnails to Search UI (elastic#104199)
9882047
@kibanamachine kibanamachine mentioned this pull request Jul 14, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jul 14, 2021
@kibanamachine @JasonStoltz
[Enterprise Search] Added Thumbnails to Search UI (#104199) (#105663)
409a0c6 
Co-authored-by: Jason Stoltzfus <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 15, 2021
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into reporting/new…
cec3989 
…-png-pdf-report-type

* 'master' of github.com:elastic/kibana: (75 commits)
  [Search Sessions] Don’t try to delete errored searches (elastic#105434)
  [Search Sessions] fix saved object can be created even if courier:batchSearches is enabled (elastic#105407)
  [Remote Clusters] Fixed remote clusters details flyout for long strings (elastic#105592)
  [ML] Functional tests - re-activate a11y tests (elastic#105198)
  [APM] Typed client-side routing (elastic#104274)
  [Canvas] Expression error (elastic#103048)
  [ML] Fixing job wizard with missing description (elastic#105574)
  [Security Solution][Alerts] - Add alerts subfeature UI (elastic#105505)
  Upgrade EUI to v35.0.0 (elastic#105127)
  [Reporting] Clean up types for internal APIs needed for UI (elastic#105508)
  skip flaky suite (elastic#105087)
  [Workplace Search] Fix Chrome issues with GitHub sources (elastic#105680)
  [Fleet] Add containerized fleet server instructions to Fleet README (elastic#105669)
  [ML] Add api integration test for analytics map endpoint  (elastic#105531)
  Fixes cypress flake across two tests (elastic#105645)
  [Logs&Metrics UI] add owner properties to plugin manifest (elastic#105580)
  chore(NA): introduce preset for jest-integration tests on @kbn/test (elastic#105144)
  [Enterprise Search] Added Thumbnails to Search UI (elastic#104199)
  Translate App Search credentials list (elastic#105619)
  [APM] APM agent config created prior to Fleet migration is not injected into integration policy (elastic#105504)
  ...

# Conflicts:
#	x-pack/plugins/reporting/common/types.ts
#	x-pack/plugins/reporting/public/management/report_listing.test.tsx
@elasticmachine
Copy link
Contributor

Pinging @elastic/app-search-frontend (Team:AppSearch)

@JasonStoltz JasonStoltz deleted the thumbnail-support branch September 8, 2021 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daveyholler daveyholler daveyholler approved these changes

@cee-chen cee-chen cee-chen approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:enhancement Team:EnterpriseSearch v7.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@JasonStoltz @cee-chen @kibanamachine @elasticmachine @daveyholler @chriscressman @KOTungseth