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

[5.0] b/c plugin - Load class aliases earlier and move es5 assets #41202

Merged
merged 9 commits into from
Jul 22, 2023

Conversation

HLeithner
Copy link
Member

This PR moves the class alias loading to the constructor function of the plugin to load the alias at the earliest possible execution point.

Additional this PR moves all es5 assets from all joomla.asset.json into the es5.asset.json and make loading them optional.

Summary of Changes

  • Move class aliasing to the constructor
  • Move es5 webassets to the plugin

Testing Instructions

Full CMS with and without plugin active

Actual result BEFORE applying this Pull Request

Should work

Expected result AFTER applying this Pull Request

Should still work

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev labels Jul 21, 2023
@HLeithner
Copy link
Member Author

Do you have anything to say @SharkyKZ or would you only like to spread bad vibes?

@@ -5,5 +5,7 @@

PLG_COMPAT_FIELD_CLASSES_ALIASES_LABEL="Classes Aliases"
PLG_COMPAT_FIELD_CLASSES_ALIASES_DESCRIPTION="Add class aliases for classes which have been renamed or moved to a namespace."
PLG_COMPAT_FIELD_ES5_ASSETS_LABEL="ES5 Assets"
PLG_COMPAT_FIELD_ES5_ASSETS_DESCRIPTION="Load the es5 webasset stubs."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this string be less technical and geeky?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps explaining when/why you might need to enable it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it and I didn't came to a better text, in the end you have to understand what es5 javascript files are... so it's more a template developer setting and not really for end user useful. But I'm happy about a better description

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something along the lines of "This should be enabled if you get an error message saying ..."

HLeithner and others added 2 commits July 21, 2023 16:49
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
@brianteeman
Copy link
Contributor

I see that there are still es5 references in
administrator\templates\atum\joomla.asset.json
build\media_source\com_joomlaupdate\joomla.asset.json
build\media_source\plg_system_jooa11y\joomla.asset.json

@HLeithner HLeithner requested a review from chmst as a code owner July 21, 2023 14:58
@HLeithner
Copy link
Member Author

I see that there are still es5 references in administrator\templates\atum\joomla.asset.json build\media_source\com_joomlaupdate\joomla.asset.json build\media_source\plg_system_jooa11y\joomla.asset.json

Thanks, don't know why my initial search didn't found the 3 assets.... in the build folder... the template I didn't checked tbh

@brianteeman
Copy link
Contributor

I just grepped for es5 in all joomla.asset.json files

@brianteeman
Copy link
Contributor

Does this need changing as well

"name": "bootstrap.es5",
"type": "script",
"deprecated": true,
"uri": "",
"dependencies": [
"core"
],
"attributes": {
"nomodule": true,
"defer": true
}

@HLeithner
Copy link
Member Author

Does this need changing as well

"name": "bootstrap.es5",
"type": "script",
"deprecated": true,
"uri": "",
"dependencies": [
"core"
],
"attributes": {
"nomodule": true,
"defer": true
}

I'm not sure I have seen it, @Fedik @dgrammatiko ?

@Fedik
Copy link
Member

Fedik commented Jul 21, 2023

All what in settings will be in vendor/assets.json
When you added it in to the legacy.json, then remove it from settings.

@HLeithner
Copy link
Member Author

thanks moved it to the plugin

Co-authored-by: Brian Teeman <brian@teeman.net>
@HLeithner HLeithner merged commit f396cd9 into joomla:5.0-dev Jul 22, 2023
3 checks passed
@HLeithner
Copy link
Member Author

Hopefully this solve all the JPlugin issues, merging it to get it into alpha3

GeraintEdwards pushed a commit to GeraintEdwards/joomla-cms that referenced this pull request Aug 14, 2023
…omla#41202)

* Move classmap import to the plugin constructor

* Move es5 assets to b/c plugin

---------

Co-authored-by: Brian Teeman <brian@teeman.net>
GeraintEdwards pushed a commit to GeraintEdwards/joomla-cms that referenced this pull request Aug 14, 2023
…omla#41202)

* Move classmap import to the plugin constructor

* Move es5 assets to b/c plugin

---------

Co-authored-by: Brian Teeman <brian@teeman.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants