?
avatar uglyeoin
uglyeoin
4 Dec 2015

Steps to reproduce the issue

Install and activate the Joomla! breadcrumbs module. Visit a page. Here is a demo website with it installed.
http://eoindemo.demojoomla.com/index.php/bread-crumbs-content

Expected result

Breadcrumbs with the class "active" on the active breadcrumb (page).

Actual result

Active Breadcrumb is active, but text saying "you are here" is also active. The first li does not need the class active on it (this is hard coded and never changes to inactive). It's easy to undo with a HTML override, but it should be part of the core, it requires minimal testing and the change took me 20 seconds.

Here is a picture to better explain it.

screen shot 2015-12-04 at 10 22 43

System information (as much as possible)

Additional comments

If I could upload a .zip file I would provide this fix for you, since I can't you can locate the issue here.

mod_breadcrumbs/tmpl/default.php

line 17

change li class="active" to li (remove the class)

avatar uglyeoin uglyeoin - open - 4 Dec 2015
avatar uglyeoin uglyeoin - change - 4 Dec 2015
The description was changed
avatar uglyeoin uglyeoin - change - 4 Dec 2015
The description was changed
avatar bertmert
bertmert - comment - 4 Dec 2015

If I could upload a .zip file I would provide this fix for you, since I can't you can locate the issue here

Try a pull request! So, anybody is able to test your code changes.
Here's a description:
https://docs.joomla.org/Using_the_Github_UI_to_Make_Pull_Requests

avatar uglyeoin
uglyeoin - comment - 7 Dec 2015

Thanks Bertmert. Thinking about it, this change may require a change to the standard Joomla! template too. I will test that and do two pull requests if need be.

avatar wojsmol
wojsmol - comment - 16 Jan 2016

@uglyeoin Any progress on this issue?

avatar infograf768
infograf768 - comment - 16 Jan 2016

I agree on the issue. But indeed this would require an override for Protostar and may change some templates display which do not use an override. Enough to qualify as not B/C, that is the question.

avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

I don't know @infograf768 I'm kind of a noob to this stuff. @wilsonge can you provide information as to whether this is considered breaking b/c? It is more likely to be templates that suffer than extensions (although not completely impossible). I would be happy to submit a pull request for the core as well as any template changes for Protostar. Is it possible to submit both?

avatar wilsonge
wilsonge - comment - 18 Jan 2016

I think it's fine to do. I'd just push it into 3.5 to be safe.

avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

@wilsonge Just to avoid additional work, Protostar will remain in J3.5?

avatar wilsonge
wilsonge - comment - 18 Jan 2016

Yes

avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

Ok I'll try to understand a pull request and submit one today

avatar wilsonge
wilsonge - comment - 18 Jan 2016

ty!

avatar wojsmol
wojsmol - comment - 18 Jan 2016
avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

@wojsmol Thanks, I assume by ping you mean just @you ??

avatar wojsmol
wojsmol - comment - 18 Jan 2016

@uglyeoin yes :smile:

avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

@wojsmol many thanks, I'll see how I go (expect me to ask for help lol)

avatar uglyeoin uglyeoin - reference | 21dba53 - 18 Jan 16
avatar uglyeoin uglyeoin - reference | 975402f - 18 Jan 16
avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

Here are my pull requests:

Joomla! Core (breadcrumbs module)
#8929

Joomla! Core (protostar template)
#8930

@wojsmol I can't believe how easy that was!

avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

@bertmert thanks for the link it was very useful

avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

@wojsmol you might want to check I did everything correctly at my end :)

avatar wilsonge
wilsonge - comment - 18 Jan 2016

PR in itself looks fine to me (haven't checked it actually works)

avatar uglyeoin
uglyeoin - comment - 18 Jan 2016

@wilsonge cheers for checking the PR. There goes the PR virginity. Whoop whoop.

avatar infograf768
infograf768 - comment - 18 Jan 2016

For #8930 I am not sure the way to go is modifying the css. I am more in favour of an override.

avatar uglyeoin
uglyeoin - comment - 19 Jan 2016

@infograf768 can you explain why you prefer an override to a less/css change? I don't mind either way it's about the same amount of work and so long as it all works at the end I am happy. I would however, prefer to do things the right way and learn for the future, anything you can do to assist is greatly appreciated.

The way I see it the override uses more files, larger files and therefore, makes the Joomla! core larger, I think adding a few characters to a less/css file is an improvement on this. Granted it is not a huge overhead but if everyone has the same attitude Joomla! will remain as light as possible.

avatar brianteeman brianteeman - change - 6 Apr 2016
Status New Closed
Closed_Date 0000-00-00 00:00:00 2016-04-06 11:10:23
Closed_By brianteeman
avatar brianteeman
brianteeman - comment - 6 Apr 2016

Closed as issue was resolved with PR


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

avatar brianteeman brianteeman - close - 6 Apr 2016
avatar brianteeman brianteeman - close - 6 Apr 2016

Add a Comment

Login with GitHub to post a comment