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

[com_finder] maps view: Indexed content counting and other improvements #10257

Merged

Conversation

andrepereiradasilva
Copy link
Contributor

@andrepereiradasilva andrepereiradasilva commented May 5, 2016

Pull Request for Improvement.

Summary of Changes

This PR adds some improvements to Smart Search Content Maps:

  1. Add a label to each content map title (it was missing)
  2. Adds link to view only the first level sublevels in the blue badges.
  3. Adds counting and corresponding links to "Indexed Content" view.
Before PR

image

After PR

image

Testing Instructions

3 simple test needed.

  1. Apply patch

  2. Go to Smart Search -> Indexed Content and run the index

  3. Go to Smart Search -> Content Maps

  4. Click on a content map "Title" and check that in that row the select box is checked.

    ok = test 1 passed

  5. Click on the blue level 1 badge. and confirm it showns only the sublevels after click. Clear the filter after.

    ok = test 2 passed

  6. Confirm the published/unpublished counting is working properly and they link to the index content view with the corresponding filter.
    Please note that the content_map will only be activated if PR [com_finder] index view: Add content map filter #10256 is also installed, but you can check the resulting URL (check the filter is there) for this test propose.

    ok = test 3 passed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels May 5, 2016
@grhcj
Copy link

grhcj commented May 8, 2016

I have tested this item 🔴 unsuccessfully on e19edeb

With this PR applied, I get
Notice: Undefined property: stdClass::$num_nodes in [...]\administrator\components\com_finder\views\maps\tmpl\default.php on line 134
for every element at content maps view.
I cleared my browser cache and tested with latest staging.


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

@andrepereiradasilva
Copy link
Contributor Author

ok will check that.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @grhcj


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

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @grhcj


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

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @grhcj


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

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @grhcj


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

@andrepereiradasilva
Copy link
Contributor Author

ok should be fixed now.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @grhcj


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

@andrepereiradasilva
Copy link
Contributor Author

@grhcj please retest

@grhcj
Copy link

grhcj commented May 8, 2016

I have tested this item ✅ successfully on ed45c32

In test 3, the filter param was present in the URL, but I can't verify that the filter value is right.
Because this shouldn't be part of this PR, I didn't do further tests on this.


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

@andrepereiradasilva
Copy link
Contributor Author

thsnk for testing, yes that will only work with #10256 also applied

@chrisdavenport
Copy link
Contributor

I have tested this item ✅ successfully on ed45c32

Works as described now that #10256 has been merged. Nice improvement.


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

@brianteeman
Copy link
Contributor

RTC -before merging please double check the unrelated Travis issue


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 8, 2016
@brianteeman brianteeman added this to the Joomla 3.6.0 milestone May 8, 2016
@wilsonge wilsonge merged commit 6b16be9 into joomla:staging May 8, 2016
@wilsonge
Copy link
Contributor

wilsonge commented May 8, 2016

Travis is unrelated and now fixed in staging. Merging this

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 8, 2016
@andrepereiradasilva
Copy link
Contributor Author

thanks for testing, comments and merging!

@andrepereiradasilva andrepereiradasilva deleted the com_finder_maps-improvements branch May 8, 2016 15:06
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.

None yet

6 participants