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

[4.0] Deleting obsolete template.min.js #23739

Merged
merged 2 commits into from
Feb 27, 2019
Merged

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Feb 1, 2019

Seems like this file is a leftover of earlier cleanup.

@dgrammatiko
Copy link
Contributor

You're breaking B/C here.
That code ensures that group buttons will work as they meant to...

In order to remove this part you need first this: #20549

@HLeithner
Copy link
Member

Seams that got fixed in 6a3edd0 ? because in this commit the original template.js as been deleted.

@dgrammatiko
Copy link
Contributor

@HLeithner No, that PR assumes that bootstrap.js is always loaded by the template. This won't be the case for Atum...

@wilsonge
Copy link
Contributor

wilsonge commented Feb 2, 2019

@dgrammatiko why is there only a minified file committed then? There is no way of trying to get the original back out from this i guess

@Hackwar
Copy link
Member Author

Hackwar commented Feb 2, 2019

There is no original source of that JS file anymore and the code has not been included in the template for pretty exactly a year now: #19326

@Hackwar
Copy link
Member Author

Hackwar commented Feb 2, 2019

@wilsonge we can of course restore the old file from a year ago, but considering that it has not been included for a year and nobody really noticed it, I would claim that it is not needed.

@HLeithner
Copy link
Member

@HLeithner No, that PR assumes that bootstrap.js is always loaded by the template. This won't be the case for Atum...

You reopend #20549 do you try to fix the Situation suiteable for screenreaders?

@dgrammatiko
Copy link
Contributor

@Hackwar @wilsonge go ahead and remove it. I already reopened #20549 and will patch the a11y concerns there.

FWIW my point was that @Bakual 's PR removing the js file was based on the assumption that Bootstrap.js will always be loaded, which of course is wrong as both Atum and Cassiopeia will be free from jQuery.js and Bootstrap.js

@Bakual
Copy link
Contributor

Bakual commented Feb 2, 2019

This PR indeed is fine since my PR back then removed all reference to load that file. I don't know why I missed the minified one. The code in that file was broken anyway.

FWIW my point was that @Bakual 's PR removing the js file was based on the assumption that Bootstrap.js will always be loaded, which of course is wrong as both Atum and Cassiopeia will be free from jQuery.js and Bootstrap.js

That's true, and explicitely mentioned in my PR back then:

Since Bootstrap is loaded anyway and it offers the same functionality, we can use just that one for now (until it gets eventually converted to a CE).

So yes, it's one roadblock to get Bootstrap removed from the template.

@wilsonge wilsonge merged commit a6d58bc into joomla:4.0-dev Feb 27, 2019
@wilsonge
Copy link
Contributor

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone Feb 27, 2019
wilsonge added a commit that referenced this pull request Mar 8, 2019
This directory was removed in #23739
@Hackwar Hackwar deleted the patch-37 branch April 27, 2019 07:56
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

6 participants