User tests: Successful: Unsuccessful:
Upgrade Chosen to the latest release (1.6.1)
Revert the change of class and function names to ensure full backwards compatability
Add the two documented hacks we made in the past
chosen is used throughout the cms for styling dropdown menus
Pay particular attention to the search tools and the ability to create categories, module positions and tags "on the fly" - these are both ares our hack was used for,
None
Category | ⇒ | JavaScript |
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
hum ... why was chosen "hacked" in the first place? .... (don't awnser that is just a rethorical question)
My question is: instead of "rehacking" in a new version can't joomla somehow extend default chosen.js in a new js file to make the changes joomla needs?
For what i understand the changes are this: 0ef056d
I was about to make a PR with https://github.com/okcoker/taggle.js for tags and drop chosen from there (that's for J4). That script is vanilla, tiny and it works wonderfully.
Now for the other places that chosen is required (why do we use chosen so extensively?) I have no clue if the applied joomla patch would be required but reading the code we should be able to extend by replacing the relative prototype.
why do we use chosen so extensively?
Style decision made in 2012.
@andrepereiradasilva now that I spend some time on it (reading the code) I realise that we are about to fork chosen since we keep the old class names, so I guess this approach of extending is not valid anyway
ok then
Hacking it was a mistake. But then we were screwed when they changed all
the class names anyway.
Discussing changing from chosen is not for this PR and cant be done until 4.0
Tests for this PR would be more productive at this time
This PR is fine in principle for the 3.x branch and the PLT approved this change before Brian made the PR. As we're changing pretty much every function because of the classes and events changes in chosen 1. So there's no point in creating a wrapper.
I think we all know that chosen will be dropped in Joomla 4 - or at least substantially reduced in scale. But 3.x isn't going to go away any time soon and this PR is a good improvement until then.
Ok will test when have time
hum ... why was chosen "hacked" in the first place?
Good question. I'm going to try and prevent this from happening in J4.x and throw abuse at anyone that even hints at hacking any JS/CSS files
@brianteeman - Can I test this PR now?
Yes it is ready for testing
I dont get that in chrome but I can confirm it in firefox - however can you
confirm if this is present before applying this patch. It looks like it is
not related as the debugger in FF doesnt highlight the chosen js and I see
it on none patched sites. (I think it is related to update check)
Ah yes sorry, my bad. This error appears when you start clicking anywhere on the page, so it's not related.
Again - is that css "error" present before this PR
No, only after the patch
I can confirm that @c-lodder but those css fixes are above my pay grade and skill level
@brianteeman - Remove box-shadow: 0 1px 0 #fff inset;
Category | JavaScript | ⇒ | Front End Components JavaScript |
Nice.
What are the PHP changes you've added doing?
Shouldnt be there - oops
On 25 August 2016 at 14:18, Lodder notifications@github.com wrote:
Nice.
What are the PHP changes you've added doing?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11779 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8VyILpu_yOnaDxFaSKbvHqFR2Frgks5qjZYygaJpZM4JsMWq
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Category | JavaScript Front End Components | ⇒ | JavaScript |
I have tested this item
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC. Thanks
Labels |
Added:
?
|
@brianteeman
I personally don't care but found a small issue. If you think that should be fixed:
Firefox 48
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-08-31 23:05:00 |
Closed_By | ⇒ | wilsonge |
Labels |
Removed:
?
|
Woot. Thank you
On 1 Sep 2016 12:05 a.m., "George Wilson" notifications@github.com wrote:
Merged into 3.7 with 08f116c
08f116c—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11779 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8VZKw_nek_lxTnZUENyH5FV6bloSks5qlgidgaJpZM4JsMWq
.
It might be a good idea if someone double checks the PRs that hacked chosen in the first place to make sure I got everything as I only ported across changes that were documented in the source code.