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_fields] Handle tag items properly #19006

Merged
merged 2 commits into from Dec 18, 2017

Conversation

laoneo
Copy link
Member

@laoneo laoneo commented Dec 6, 2017

Pull Request for Issue #18992.

Summary of Changes

Prepares an item in the tags view with the custom fields. It mapps the tag context to the propper one of the item which has the tag.

Testing Instructions

  • Create an article custom field.
  • Create an article with a value of the custom field and give it a tag.
  • Create a menu item "Tagged Items" and select the tag you gave the article, also set "Item Description" to Show as on the screenshot below
    image
  • Open the menu on the front

Expected result

The custom fields are shown on the item.

Actual result

No custom fields are shown.

@ghost
Copy link

ghost commented Dec 7, 2017

@shionphan can you please test?

@ghost
Copy link

ghost commented Dec 7, 2017

I have tested this item 🔴 unsuccessfully on 6056d1d

Field (Type: Text, using Default Settings) isn't shown applied PR.


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

@laoneo
Copy link
Member Author

laoneo commented Dec 7, 2017

@franz-wohlkoenig what shows up on the front? Did you not set a value ion the article for the custom field?

@ghost
Copy link

ghost commented Dec 7, 2017

In Frontend nothing new (=Field-Value) is shown. Value in Field is set.

@laoneo
Copy link
Member Author

laoneo commented Dec 7, 2017

Can you post a screenshot of your page? Are you sure you took the right menu item type?

@ghost
Copy link

ghost commented Dec 7, 2017

You see a little more Space between Title and Text using PR.

Without PR

without

With PR

with

@laoneo
Copy link
Member Author

laoneo commented Dec 7, 2017

What's in the space, when you inspect it with the developer tools of your browser?

@ghost
Copy link

ghost commented Dec 7, 2017

bildschirmfoto 2017-12-07 um 11 30 08

@laoneo
Copy link
Member Author

laoneo commented Dec 7, 2017

So a field is loaded which means it works. It can be an override or another patch installed which interferes on your site. Lets see if others have the same behavior.

@ghost
Copy link

ghost commented Dec 7, 2017

If other have not same Behaviour i install new one.

@esedic
Copy link
Contributor

esedic commented Dec 7, 2017

I also can't get custom fields to display.

<dl class="fields-container"></dl>

is created but it's empty.

@ghost
Copy link

ghost commented Dec 7, 2017

Same Behaviour on clean Install.

@shionphan
Copy link

shionphan commented Dec 8, 2017

@franz-wohlkoenig @laoneo

I have test code <?php echo $item->event->beforeDisplayContent; ?> successfull.
It seems great. Thanks all.

Screenshot was script console.log

image

Issue

Some fields can not display.

Others fields what be set default value could be displaed.

Possible Causes

Find code $fields = FieldsHelper::getFields($context, $item, true); in /plugins/system/fields/fields.php

Add console code

$console = json_encode($fields);
echo ('<script>console.log(' . $console . ')</script>');

Some field value In article page will be displayed, But it dose not display in com_tags page.

@ghost
Copy link

ghost commented Dec 8, 2017

@shionphan please mark your Test as successfully:

  • open Issue Tracker
  • Login with your github-Account
  • Click on blue "Test this"-Button above Authors-Picture
  • mark your Test as successfully
  • hit "submit test result"

@laoneo
Copy link
Member Author

laoneo commented Dec 8, 2017

Just tested it and the custom fields values are shown as expected.
image

Can you guys exactly describe what for steps you did where the field is not displayed. It would be helpfull if you can test with the latest staging code and from a fresh joomla installation.

@ghost
Copy link

ghost commented Dec 8, 2017

System information

3.8.3-rc
Multilanguage Site (4 Lang.) & Sample Data
macOS Sierra, 10.12.6
Firefox 57 (64-bit)

MAMP 4.2

  • PHP 7.0.22
  • MySQLi 5.6.35

Create new Field

bildschirmfoto 2017-12-08 um 09 26 15

Article: Value on Field and Tag

bildschirmfoto 2017-12-08 um 09 29 23

Menu "Tags » Tagged Items" using Tag and Item Description to show

bildschirmfoto 2017-12-08 um 09 31 33

Result:

bildschirmfoto 2017-12-08 um 09 34 23

@laoneo
Copy link
Member Author

laoneo commented Dec 8, 2017

Are you testing it with the latest staging?

@ghost
Copy link

ghost commented Dec 8, 2017

I test on 3.8.3-rc, former Test nightly Builds.

@shionphan
Copy link

Tested successfull that if I install a new Joomla clean package, but it dose not work on my site.
I will rebuild database for continue to follow up.

@laoneo
Copy link
Member Author

laoneo commented Dec 10, 2017

@shionphan please fill the test instructions from Franz to mark the pr as a successful test. Thanks.

@shionphan
Copy link

I have tested this item ✅ successfully on 6056d1d

Tested successfull that if I install a new Joomla clean package


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

@shionphan
Copy link

shionphan commented Dec 14, 2017

@laoneo Sorry for the work too busy.
A new problem you have to know. Tags page will display items fields when i add new items. However, the old item data will not be displayed.

We know that fields data is different from items data in mysql (table name is ##_contentitem_tag_map.content_item_id). I can guarantee that I add the data operation is the same.

Solution

Line 423 (Your post)

$item->id       = $item->content_id;

Change into:

$item->id       = $item->content_item_id;

Demo pages

https://www.infinisign.com/products/all-type/multi-domain (Chinese Language)

@laoneo
Copy link
Member Author

laoneo commented Dec 14, 2017

@shionphan thanks for the hint, changed it. Can you guys please retest if it is working now as expected?

@ghost
Copy link

ghost commented Dec 14, 2017

I have tested this item ✅ successfully on f440255

bildschirmfoto 2017-12-14 um 09 12 11


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

@shionphan
Copy link

shionphan commented Dec 14, 2017

I have tested this item ✅ successfully on f440255

Perfect

image


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

@ghost
Copy link

ghost commented Dec 14, 2017

Ready to Commit after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 14, 2017
@mbabker mbabker added this to the Joomla 3.8.4 milestone Dec 18, 2017
@mbabker mbabker merged commit 27e4d15 into joomla:staging Dec 18, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 18, 2017
@laoneo laoneo deleted the cf/tags/mapping branch December 18, 2017 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants