Skip to content

[4.0] Reworking result layout of com_finder#20572

Merged
wilsonge merged 12 commits intojoomla:4.0-devfrom
Hackwar:j4finder_resultlayout
Jul 7, 2018
Merged

[4.0] Reworking result layout of com_finder#20572
wilsonge merged 12 commits intojoomla:4.0-devfrom
Hackwar:j4finder_resultlayout

Conversation

@Hackwar
Copy link
Member

@Hackwar Hackwar commented May 24, 2018

The result layout of com_finder is very basic and the results of com_search leave a bigger impression. If we want all that information to be presented in the search results, is another question.

This PR reworks the result layout of com_finder to look more like those of com_search. I'm not happy with the current result myself, but I wanted to bring this up for discussion. What do you guys think? Should we rework this or rather not? Or rather just remove the URL below the result? (See #20571)

@brianteeman
Copy link
Contributor

I like the idea that both search components should have the same layout. This pr is close but still not identical

Search

chrome_2018-05-25_12-33-41

Smart search

chrome_2018-05-25_12-34-38

<?php endif; ?>
<?php if ($this->result->created && $this->params->get('show_date', 1)) : ?>
<dd class="result-created">
<?php echo JText::sprintf('JGLOBAL_CREATED_DATE_ON', \JHtml::_('date', $this->result->created, \JText::_('DATE_FORMAT_LC3'))); ?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Add backslash before JText::sprintf.

Copy link
Member

Choose a reason for hiding this comment

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

Why not use from Text class? Text::sprintf

$query = $query instanceof DatabaseQuery ? $query : $db->getQuery(true)
->select('a.id, a.title, a.alias, a.introtext AS summary, a.fulltext AS body')
->select('a.state, a.catid, a.created AS start_date, a.created_by')
->select('a.state, a.catid, a.created AS start_date, a.created, a.created_by')
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to add a.created when we're already selecting it just calling it start_date?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. Reverting that one.

@carlitorweb
Copy link
Member

carlitorweb commented Jun 18, 2018

Well, I really not like the layout of com_search. I Like more the compact mode of com_finder, Title, Text and maybe the URL.
So, I test this PR and work ok, but not sure if this PR is more like an RFC or for add this reworking layout.

@Hackwar
Copy link
Member Author

Hackwar commented Jun 18, 2018

I will see what I can do to incorporate your comment @carlitorweb

@carlitorweb
Copy link
Member

carlitorweb commented Jun 18, 2018

Thank you :). Is just my opinion about it.

@carlitorweb
Copy link
Member

Some update for this one @Hackwar ?

@Hackwar
Copy link
Member Author

Hackwar commented Jun 27, 2018

I will. I just need some more time. The priority of this PR is rather low.

@carlitorweb
Copy link
Member

The priority of this PR is rather low.

Yes I know

Hackwar added 2 commits July 1, 2018 11:39
…4finder_resultlayout

# Conflicts:
#	components/com_finder/tmpl/search/default_result.php
#	components/com_finder/tmpl/search/default_results.php
@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Jul 1, 2018
@Hackwar
Copy link
Member Author

Hackwar commented Jul 1, 2018

I've updated this PR and consider this now feature-complete. I would call this an passable generalised result layout, which now can display the different taxonomies, the date of the item and the URL besides the title and the description. If somebody has a good solution to get a spacer between the search results, I'm all ears.

While implementing the taxonomies, it became apparent that we want to control which of these to display and which to hide. In theory the taxonomies support publish/unpublish states and even the access modifiers. However right now the taxonomies are stored inside the serialized FinderIndexerResult object and are not updated when retrieving the search results or when changing the taxonomies in the backend. Besides that, the taxonomies are lacking a GUI to modify the access modifiers. I plan to rework the taxonomies, too, and would address those issues there.

Happy testing.

Copy link
Contributor

@brianteeman brianteeman left a comment

Choose a reason for hiding this comment

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

approve langauge strings

@carlitorweb
Copy link
Member

BEFORE

screenshot_20180706091424

AFTER

screenshot_20180706091412

The highlighted word inside a badge no looks so good. My personal opinion, I hate that "highlight" stuff in a search result. For me, a clean result page as much as possible can be, is better for the user can find faster what they looking for.

A way I like to see a search results page is having, for example, something like this:

screenshot_20180706093417

Can be rows with 2 column or 3 depending on the layout.

@Hackwar
Copy link
Member Author

Hackwar commented Jul 6, 2018

@carlitorweb I agree sort of with you. This is definitely not the ultimate search result layout, but I guess it is the best compromise that we can have here. Anyway, thanks for testing. I will fix the conflict and then it would be nice if you could mark this as successfully tested. 😉

@carlitorweb
Copy link
Member

carlitorweb commented Jul 6, 2018

This is definitely not the ultimate search result layout

I know buddy, just was sharing with you my thought. Waiting for the commit, for put the test.

Hackwar added 2 commits July 6, 2018 19:54
…4finder_resultlayout

# Conflicts:
#	components/com_finder/tmpl/search/default_result.php
@Hackwar
Copy link
Member Author

Hackwar commented Jul 6, 2018

Conflicts fixed.

@carlitorweb
Copy link
Member

I have tested this item ✅ successfully on 4dea366


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

1 similar comment
@Quy
Copy link
Contributor

Quy commented Jul 6, 2018

I have tested this item ✅ successfully on 4dea366


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

@Quy
Copy link
Contributor

Quy commented Jul 6, 2018

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 6, 2018
@Hackwar
Copy link
Member Author

Hackwar commented Jul 7, 2018

Thank you!

…4finder_resultlayout

# Conflicts:
#	administrator/components/com_finder/config.xml
@Hackwar
Copy link
Member Author

Hackwar commented Jul 7, 2018

Fixed the conflict by the merge of #20681. Do we have to retest all of this or can we simply merge this?

@TobsBobs
Copy link

TobsBobs commented Jul 7, 2018

screenshot_2018-07-07 home

@TobsBobs
Copy link

TobsBobs commented Jul 7, 2018

I have tested this item ✅ successfully on b1787b6


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

@wilsonge wilsonge merged commit 4b1214a into joomla:4.0-dev Jul 7, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 7, 2018
@wilsonge wilsonge added this to the Joomla 4.0 milestone Jul 7, 2018
@wilsonge
Copy link
Contributor

wilsonge commented Jul 7, 2018

Thanks :)

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

Comments