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

Make better use of Hotwire #45

Merged
merged 35 commits into from
Jun 17, 2023
Merged

Make better use of Hotwire #45

merged 35 commits into from
Jun 17, 2023

Conversation

vinnydiehl
Copy link
Owner

@vinnydiehl vinnydiehl commented Jun 15, 2023

I wish I had taken more time to learn about Hotwire before I started this project, but better late than never. Goals for this PR are:

  • Rewrite all existing JavaScript to utilize Hotwire.
  • Convert all JavaScript to 2 space tab width.

fixes #6

@vinnydiehl vinnydiehl added the refactor Code needs to be reworked label Jun 15, 2023
@vinnydiehl vinnydiehl self-assigned this Jun 15, 2023
This one is really nice.
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

RFC 5322 compliant.
}

#isValidEmail(input) {
return input.match(/(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])/) != null;

Check failure

Code scanning / CodeQL

Inefficient regular expression

This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '\\'.
This got split into 2 Stimulus controllers, `ev` and `goals`.
Whoops, forgot to point slip to the new controller.
The original `cookies.js` is still in place until everything that
depends on it is definitely converted.
The controller is on the parent, this was messing things up.
This was intermittent in CI.
The button is a bit out of the scope of this branch, but I needed a
trigger to call from the Stimulus `goals#check` method.
@vinnydiehl vinnydiehl force-pushed the hotwire branch 2 times, most recently from 2fb8396 to 6302efb Compare June 17, 2023 06:49
@vinnydiehl vinnydiehl marked this pull request as ready for review June 17, 2023 08:49
@vinnydiehl vinnydiehl merged commit 908370e into trunk Jun 17, 2023
@vinnydiehl vinnydiehl deleted the hotwire branch June 17, 2023 11:36
vinnydiehl added a commit that referenced this pull request Jun 18, 2023
This `#nav_link_js` nonsense is no longer necessary after #45.

fixes #46
vinnydiehl added a commit that referenced this pull request Jun 18, 2023
This `#nav_link_js` nonsense is no longer necessary after #45.

fixes #46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code needs to be reworked
Projects
None yet
1 participant