Skip to content

[4.0] Recaptcha plugins javascript#26839

Merged
wilsonge merged 3 commits intojoomla:4.0-devfrom
dgrammatiko:4.0-dev-es6-1
Nov 26, 2019
Merged

[4.0] Recaptcha plugins javascript#26839
wilsonge merged 3 commits intojoomla:4.0-devfrom
dgrammatiko:4.0-dev-es6-1

Conversation

@dgrammatiko
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

  • Convert es5 to es6
  • Usage of Joomla object, drop the global variables (not the ones from Google)
  • CS

Testing Instructions

Both versions of captchas are still functional

Expected result

Actual result

Documentation Changes Required

No

@dgrammatiko dgrammatiko requested a review from wilsonge as a code owner October 27, 2019 12:26
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Oct 27, 2019
const itemNodes = document.getElementsByClassName('g-recaptcha');
const optionKeys = ['sitekey', 'theme', 'size', 'tabindex', 'callback', 'expired-callback', 'error-callback'];
const items = [].slice.call(itemNodes);
const elements = [].slice.call(document.getElementsByClassName('g-recaptcha'));
Copy link
Member

Choose a reason for hiding this comment

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

No need for an array slice call here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this script is targeting the front end thus is better to make sure that the es5 version will work down to IE9. Since there is the option to provide es5 scripts I think it would be better those scripts could live without the need for a polyfill...

Copy link
Member

Choose a reason for hiding this comment

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

IE9? Haha ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My point is that the ES5 scripts should reflect on the browsers that run on that version and IE9 is the oldest one that had pretty decent js engine. Also my idea here is to not put artificial barriers. Obviously I won't build a site with support for IE9 but all my deliverables are still fully functional on IE11. The idea of killing ES5 before IE11 is dead IMHO is a really bad one especially when there is a very well known and established practice that will fulfil both words (type=module, nomodule)...

@wilsonge wilsonge merged commit c849ba8 into joomla:4.0-dev Nov 26, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Nov 26, 2019
@wilsonge
Copy link
Contributor

Thanks!

@dgrammatiko dgrammatiko deleted the 4.0-dev-es6-1 branch February 17, 2020 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants