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

[4.1] Images in the Smart Search results #35612

Merged
merged 44 commits into from
Oct 24, 2021
Merged

Conversation

sakiss
Copy link
Contributor

@sakiss sakiss commented Sep 20, 2021

Summary of Changes

The PR allows the use of images in the finder's results.

A new setting was added named "Result Image" both in the com_finder's (aka Smart Search) configuration and it's respective "search" menu item.

Testing Instructions

  1. Go to the "Smart Search" component, press the "Clear Index" button and then the "Index".
  2. Press the "Options" button to load the configuration and enable the "Result Image" setting.
  3. Use the smart search in the site's front-end and check the returned articles that have "Intro Image" set.

Actual result BEFORE applying this Pull Request

result_w_images

Expected result AFTER applying this Pull Request

result_images

Sidenotes

Atm only the com_content finder plugin uses the images API function. Support for the rest plugins will be added in a next PR.

There are 3 new settings both in Smart Search Configuration and in Smart Search results menu item:
result_images_params

  1. Result Image > Shows or hides the image in the results.
  2. Image Class > Adds a class to the figure html element. The same that happens with the "Intro Image Class" in com_content.
  3. Linked Image > Creates an anchor for the image. The same that happens with the "Linked Intro Image" in com_content.

Documentation Changes Required

DKN

@ceford @mateoAdi @universewrld it would be great if you could test that again, as i had to close the other PR for that feature.

sakiss and others added 30 commits September 20, 2021 15:10
This reverts commit 8731d82
@mateoAdi
Copy link

I have tested this item ✅ successfully on a22bd51


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35612.

Copy link
Member

@Fedik Fedik left a comment

Choose a reason for hiding this comment

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

This still need to be fixed #35612 (comment)

@sakiss
Copy link
Contributor Author

sakiss commented Oct 7, 2021

@Fedik Done. Plz remove the "Change Requested" label.
Let's hope that this gets 1 more test and finally got merged.

Copy link
Member

@Fedik Fedik left a comment

Choose a reason for hiding this comment

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

Looks much better, thanks

@RickR2H
Copy link
Member

RickR2H commented Oct 10, 2021

I have tested this item ✅ successfully on a22bd51

Nice addition to the result page!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35612.

@RickR2H
Copy link
Member

RickR2H commented Oct 10, 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35612.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 10, 2021
@sakiss
Copy link
Contributor Author

sakiss commented Oct 22, 2021

RTC?

@RickR2H
Copy link
Member

RickR2H commented Oct 22, 2021

Hey Sakis, RTC means Ready To Commit. Next it is up to the release lead to decide if it's a good addition to the platform. If so, they will merge the PR. @bembelimen could you take a look at this PR?

@sakiss
Copy link
Contributor Author

sakiss commented Oct 22, 2021

@RickR2H the question goes to the release leader

@RickR2H
Copy link
Member

RickR2H commented Oct 22, 2021

RL is tagged and PR has been noted!

@bembelimen bembelimen merged commit 0029528 into joomla:4.1-dev Oct 24, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 24, 2021
@bembelimen bembelimen added this to the Joomla 4.1 milestone Oct 24, 2021
@bembelimen
Copy link
Contributor

Thx

@sakiss
Copy link
Contributor Author

sakiss commented Oct 25, 2021

Thanks!

@brianteeman
Copy link
Contributor

Can someone please reconfirm that this functionality works as I cant get it show any images at all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants