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

Feature/cc 409 admin dejquery #685

Open
wants to merge 50 commits into
base: release280
Choose a base branch
from

Conversation

tw2113
Copy link
Member

@tw2113 tw2113 commented Sep 24, 2024

This PR removes MOST of the jQuery integration that we had with the admin side of our plugin interaction and functionality.

There are a couple spots that rely on CMB2 jQuery-based events that I had to leave in place with the builder.js file. If we decide long term to move away from CMB2, we can address at that time, but that is a much larger project than what we presently do.

There is also a spot using jQuery UI Dialog with the modal.js file regarding log deletion confirmation that we're leaving as jQuery for the time being.

Only the files in the assets/js/ctct-plugin-admin/ need to be reviewed, the others are build files, sourced from that folder.

tw2113 added 30 commits July 30, 2024 19:48
…at refers to a "connection-settings-redirect" html class, I think this small part of the javascript is orphaned.

On top of that, I checked out the 2.0.0 branch when it was first introduced, and still couldn't find any references to the class. Given the scope and size of the 2.0.0 release, I
must conclude that the javascript spot was originally implemented at one time during the 2.0.0 release work, but a pivot was made and the js was missed for removal afterwards.

I have tested the connection process without this snippet in the build files, and our workflow remained unaffected. If by chance we need to re-add in, after a release and support
comes in, we most definitely can.
* early versions of new notification for urging to update the plugin.

* new notification setup regarding pending updates

* revise notification content to include links to the update page

* fix typo

* early return for non-admins

* remove unneeded comparisons

* early return for non CTCT sections
@tw2113 tw2113 changed the base branch from main to release270 September 24, 2024 16:19
@tw2113
Copy link
Member Author

tw2113 commented Sep 24, 2024

If we run into build file issues, merge anyway and we can re-run npm scripts afterwards.

@tw2113 tw2113 added this to the 2.7.0 milestone Sep 24, 2024
* move our missed api request method to public.

* remove is_connected check and target our list array index more directly

* begin checking if we have a list, but are not connected at the moment, and attempt to save the record

* construct a contact array to match existing code

* new methods to help with email notifications about API issues. Making this more direct and not dependent on logging in to see

* colaborated wordsmithing with Constant Contact

* fill in version number
* filter out woocommerce specific lists from the forms plugin

* this is not serving any purpose

* return early if we do not have any lists

* while we should have a list item for each, do an empty check before trying to add to array

* wp core get_option only has 2 parameters available to it.
* remove old comments

* build files

* hide optin if we have no lists available but are connected

* build files
@lindseywb
Copy link

Great work so far! The common thing that I see is getting the let vs const issues fixed.

@tw2113
Copy link
Member Author

tw2113 commented Oct 21, 2024

@lindseywb regarding let vs const are you thinking pick one and stick to that keyword as much as possible? Or perhaps "any that aren't getting re-assigned ever, go with const. If they are being re-assigned later, no problem with let" ?

@lindseywb
Copy link

@tw2113 if the value is not being reassigned, they should be const. Otherwise, they should be let.

@tw2113 tw2113 requested review from lindseywb and removed request for guzmandrade-dev October 22, 2024 17:53
@tw2113 tw2113 removed this from the 2.7.0 milestone Nov 13, 2024
@tw2113 tw2113 changed the base branch from release270 to release280 November 18, 2024 14:15
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.

2 participants