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

Replace jQuery for hide and show statements selected by ID #917

Merged
merged 4 commits into from
Nov 16, 2022

Conversation

Jaifroid
Copy link
Member

This is initial work on #916.

So far, it only deals with simple substitution of jQuery statements with their native DOM equivalents for hide and show selected by ID in app.js.

@Jaifroid Jaifroid added cleanup remove-jquery Issues or PRs involving removal of jQuery labels Nov 14, 2022
@Jaifroid Jaifroid added this to the v3.7 milestone Nov 14, 2022
@Jaifroid Jaifroid self-assigned this Nov 14, 2022
@Jaifroid
Copy link
Member Author

Added uiUtil.js.

For the record, these regexes do the trick (Perl):

  • Hide: $subject =~ s/\$\((['"])#([^\1\n]+?)\1\)\.hide\(\)/document.getElementById('$2').style.display = 'none'/sg;
  • Show: $subject =~ s/\$\((['"])#([^\1\n]+?)\1\)\.show\(\)/document.getElementById('$2').style.display = ''/sg;

Copy link
Contributor

@mossroy mossroy left a comment

Choose a reason for hiding this comment

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

I did not test but it seems to be the good approach to me (bonus point for the regexp ;-) )

@Jaifroid
Copy link
Member Author

Because jQuery handles errors (e.g. if an element doesn't exist but we try to set its style property), we need to be sure to test any transient elements that we are hiding or showing if there is any possibility of their being removed from the DOM or not present (yet) in the DOM. There should only be a very small number of these (possibly only the one pushed in da80bcf). Needs careful checking.

@Jaifroid Jaifroid marked this pull request as ready for review November 16, 2022 15:51
@Jaifroid
Copy link
Member Author

I tested on Firefox OS simulator and IE11 (as well as Edge Chromium), as these are most likely to be affected by removal of jQuery. No problems found.

@Jaifroid Jaifroid merged commit 39051c9 into master Nov 16, 2022
@Jaifroid Jaifroid deleted the Remove-jQuery-hide-and-show-statements-for-ID branch November 16, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup remove-jquery Issues or PRs involving removal of jQuery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants