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

Display category title as page heading and page title when no menu item for com_content category #19195

Merged
merged 3 commits into from Jan 15, 2018

Conversation

Projects
None yet
7 participants
@infograf768
Member

infograf768 commented Dec 28, 2017

Pull Request for Issue #19192

Summary of Changes

Patch similar to #12167
Limited to com_content

Steps to reproduce the issue

Create a menu item with the title "Categories" that lists all the categories, when accessing the menu, the page title in the browser shows "Categories" and if I click on any category the browser title does not change.

Expected result

When accessing a category of the list, the title of the category should be displayed in the browser.

Actual result

When accessing a category of the list it continues to show the menu title instead of the title of the category that has been selected .

After patch

The category title will display
Example:
screen shot 2017-12-28 at 10 06 35

Documentation Changes Required

@infograf768

This comment has been minimized.

Member

infograf768 commented Dec 28, 2017

Although this can be useful, it looks like it does not really solve #19192

@infograf768

This comment has been minimized.

Member

infograf768 commented Dec 28, 2017

@Bakual
It looks like we do get the correct $title with the PR (did a var_dump line 219), but that the Head is reloaded and displays the former menu title.

@joomdonation

This comment has been minimized.

Contributor

joomdonation commented Dec 28, 2017

@infograf768 The page title is set again in parent class, see https://github.com/joomla/joomla-cms/blob/staging/libraries/src/MVC/View/CategoryView.php#L271

So if you want to set title, instead of calling

$this->document->setTitle($title);

You need to call

 $this->params->set('page_title', $title);

However, I think the logic is more complicated than that. If the active menu item is linked to current category, then page title setting from menu item should be used instead of hard code to category title

(just comment base on quick look at the code, so I might be wrong)

@infograf768

This comment has been minimized.

Member

infograf768 commented Dec 28, 2017

tks, will look later on.

@infograf768

This comment has been minimized.

Member

infograf768 commented Dec 28, 2017

@joomdonation
By checking for view=categories&id= I got sure the category name is only displayed when there is no specific menu item for that category.
Below, Fruit Shop Site, has no menu item.
cat_title

@infograf768 infograf768 changed the title from Display category title as page heading when no menu item for com_content category to Display category title as page heading and page title when no menu item for com_content category Dec 28, 2017

@segundochinguel

This comment has been minimized.

segundochinguel commented Dec 28, 2017

@infograf768
I saw the gif and it does exactly what I was looking for. I have not implemented it yet, but I trust that was the solution. I had trying to overwrite view.html.php in category, but without results. You may not close the case until you try it in a few hours.

@infograf768

This comment has been minimized.

Member

infograf768 commented Dec 29, 2017

@segundochinguel

This comment has been minimized.

segundochinguel commented Dec 29, 2017

I have tested this item successfully on 2c1afba


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

1 similar comment
@Quy

This comment has been minimized.

Contributor

Quy commented Jan 9, 2018

I have tested this item successfully on 2c1afba


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

@Quy

This comment has been minimized.

Contributor

Quy commented Jan 9, 2018

RTC

@joomla-cms-bot joomla-cms-bot added the RTC label Jan 9, 2018

@infograf768 infograf768 added this to the Joomla 3.8.4 milestone Jan 9, 2018

if ($this->category->title && strpos($menu->link, 'view=categories&id='))
{
$this->params->def('page_heading', $this->category->title);
$title = $title ?: $this->category->title;

This comment has been minimized.

@joomdonation

joomdonation Jan 9, 2018

Contributor

Before this code, $title is null, so I I think this line should be changed to $title = $this->category->title;

{
$this->params->def('page_heading', $this->params->get('page_title', $menu->title));
$title = $title ?: $this->params->get('page_title', $menu->title);

This comment has been minimized.

@joomdonation

joomdonation Jan 9, 2018

Contributor

Same with above, this line should be changed to $title = $this->params->get('page_title', $menu->title);

@joomdonation

This comment has been minimized.

Contributor

joomdonation commented Jan 9, 2018

If it is possible, I think the code should be changed to:

if ($menu
	&& $menu->component == 'com_content'
	&& isset($menu->query['view'], $menu->query['id'])
	&& $menu->query['view'] == 'category'
	&& $menu->query['id'] == $this->category->id)
{
	$this->params->def('page_heading', $this->params->get('page_title', $menu->title));
	$title = $this->params->get('page_title', $menu->title);
}
else
{
	$this->params->def('page_heading', $this->category->title);
	$title = $this->category->title;
	$this->params->set('page_title', $title);
}

Look a bit longer but it clearly show that the if block is for case current menu item is direct link to the current displayed category.

@segundochinguel

This comment has been minimized.

segundochinguel commented Jan 9, 2018

@joomdonation, I tried this code correctly.

Now, I guess this replaces the previously accepted code.

@infograf768

This comment has been minimized.

Member

infograf768 commented Jan 10, 2018

Modified to use @joomdonation proposal. Indeed looks cleaner. 👍
@segundochinguel
@Quy
Please test and mark OK on https://issues.joomla.org/tracker/joomla-cms/19195

@Quy

This comment has been minimized.

Contributor

Quy commented Jan 10, 2018

I have tested this item successfully on e72d97a


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.8.4 milestone Jan 14, 2018

@franz-wohlkoenig

This comment has been minimized.

franz-wohlkoenig commented Jan 14, 2018

Ready to Commit after two successful tests.

@mbabker mbabker added this to the Joomla 3.8.4 milestone Jan 15, 2018

@mbabker mbabker merged commit a905334 into joomla:staging Jan 15, 2018

4 of 5 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!

@joomla-cms-bot joomla-cms-bot removed the RTC label Jan 15, 2018

@infograf768 infograf768 deleted the infograf768:cat_title_content branch Jan 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment