Skip to content

Comments

[4.0] Remove jQuery and boostrap.js deps for templates (redo of #23742)#27473

Merged
wilsonge merged 2 commits intojoomla:4.0-devfrom
Fedik:rem-jq-bs-deps
Jan 15, 2020
Merged

[4.0] Remove jQuery and boostrap.js deps for templates (redo of #23742)#27473
wilsonge merged 2 commits intojoomla:4.0-devfrom
Fedik:rem-jq-bs-deps

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented Jan 11, 2020

Summary of Changes

Remove jQuery and boostrap.js dependencies for a templates.
Partial redo of #23742, that lost somewhere while waiting for #25775

Testing Instructions

Apply patch, and make sure all still works.

Expected result

Works

Actual result

Works

Documentation Changes Required

nope

@richard67
Copy link
Member

richard67 commented Jan 11, 2020

@Fedik I've just tested as described and everything works. The only thing which confuses me a bit is that if I am in backend or in frontend (but in frontend only after having logged in) I can see in network analysis of browser development tools that jquery.min.js and bootstrap.bundle.min.js are still loaded. Is that expected? If yes, I'll give a good test result. Just let me know.

Update: Or does it need some special action after having applied the patch e.g. with patchtester?

@Fedik
Copy link
Member Author

Fedik commented Jan 11, 2020

The jquery.min.js and bootstrap.bundle.min.js still may be loaded, because they requested by other components, layouts etc.

The main idea in the patch is that the template does not require these scripts to work, so they should not be set as dependency.
You can find some more details in #23742

@richard67
Copy link
Member

@Fedik Yes, I had already checked #23742 . I just have asked because I wanted to be sure all is right.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 3917a77

All works as well as before after having applied the patch and watching browser console errors and php error logs while checking all.

In addition, the backend and frontend templates don't load bootstrap and jquery anymore when not being logged in.

Only after login when some component requires that, they are loaded.


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

@richard67
Copy link
Member

And to make it complete: Drone failure as usual seems not to be related to this PR.

@SharkyKZ
Copy link
Contributor

This breaks menu in frontend. See similar PR #23744.

@richard67
Copy link
Member

@SharkyKZ Am just testing again but can't see what's broken. Could you provide some details?

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Jan 11, 2020

The menu toggle button on small screens doesn't work:

Screen Shot 2020-01-11 at 13 39 03

@Fedik
Copy link
Member Author

Fedik commented Jan 11, 2020

@SharkyKZ it seems on mobile version, I did not noticed,
Thanks, I will check

@richard67
Copy link
Member

Ouch, didn't notice that.

@richard67
Copy link
Member

I have not tested this item.


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

@Fedik
Copy link
Member Author

Fedik commented Jan 11, 2020

Okay I have add bootstrap.js back to cassiopeia, it seems the mobile menu use "navbar-toggler" feature.
Should work now.

@richard67
Copy link
Member

I have tested this item ✅ successfully on d7df7b4


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

1 similar comment
@jwaisner
Copy link
Member

I have tested this item ✅ successfully on d7df7b4


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 11, 2020
@dgrammatiko
Copy link
Contributor

Thank you @Fedik 🙌

@wilsonge wilsonge merged commit 78e51d2 into joomla:4.0-dev Jan 15, 2020
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 15, 2020
@wilsonge wilsonge added this to the Joomla 4.0 milestone Jan 15, 2020
@Fedik Fedik deleted the rem-jq-bs-deps branch January 15, 2020 20:15
@Quy
Copy link
Contributor

Quy commented Jan 16, 2020

It is no longer possible to tab between Set 2, Set 1, and Set 0 in the TinyMCE plugin.

@Fedik
Copy link
Member Author

Fedik commented Jan 17, 2020

need to load bootstrap in that specific layout

@dgrammatiko
Copy link
Contributor

Or use the custom elements tabs as the rest of the backend?

@Fedik
Copy link
Member Author

Fedik commented Jan 17, 2020

when it will stop mess with DOM 😉

@dgrammatiko
Copy link
Contributor

When will it adhere to the standards for CE? (I have no answer for both questions, it seems both issues are due to my bad implementation)

@richard67
Copy link
Member

It is no longer possible to tab between Set 2, Set 1, and Set 0 in the TinyMCE plugin.

@Quy Seems I've missed that when testing, shame on me.

Will someone make a new issue for that, or even a new PR?

@Fedik
Copy link
Member Author

Fedik commented Jan 17, 2020

@richard67 I will do PR, but some time later, maybe on the weekend

@Fedik
Copy link
Member Author

Fedik commented Jan 18, 2020

@richard67 @Quy please test this one #27560

brianteeman pushed a commit to brianteeman/joomla-cms that referenced this pull request Feb 4, 2020
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.

8 participants