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/rewrite plugin webpack #79

Merged
merged 29 commits into from Jan 26, 2018

Conversation

Projects
None yet
3 participants
@dingo-d
Member

dingo-d commented Nov 1, 2017

Ok so this is quite a big rewrite. In the light of core ticket #40894 I wanted to rewrite the plugin using webpack and more modern ES6 syntax of JS. Plus I wanted to separate business from presentational logic.

It is quite a bit away from your old plain jQuery, but I think that it's understandable enough and that it can encourage others in the WordPress community to start working with modern JS tools :)

I've made several modifications:

  • Updated the readme with the build process
  • Updated package.json with additional information, and dependencies
  • Added ES linter
  • Added babel for backward compatibility (needs testing on IE11)
  • Cleaned up the code a bit, added proper messages that should be in the ajax response if something is wrong
  • Changed POST to GETrequests in the callbacks, since we are not writing anything to the database - it's semantically more correct

To do:

  • Replace css with sass in another PR

dingo-d added some commits Oct 24, 2017

Refactor js code, add linting, webpack and all that jazz
Need to change php files so that they work with the newly refactored js code.
JS and php fixes
Fixes that should accomodate the changes in the php code because of the changes in JS code. Minor build settings changes.
Changes to the code
Still need to fix quirks with files, add stop button (add listeners for start and stop for the ajax).
Fix the report add disable on button
Need to add the ajax stop functionality and make final refactor check
@grappler

This looks great. Thank you for the PR. Will try it out later.

$theme_slug = sanitize_text_field( wp_unslash( $_GET['themeName'] ) );
if ( isset( $_GET['hideWarning'] ) && 'true' === $_GET['hideWarning'] ) {
$args['show_warnings'] = 'false'; // MANUAL INSPECTION!!!!!!!

This comment has been minimized.

@grappler

grappler Nov 1, 2017

Member

What is the reason for the code comment?

This comment has been minimized.

@dingo-d

dingo-d Nov 1, 2017

Member

Oh, my bad, forgot to remove this comment. I needed to check these $_GET variables. Will remove this.

$headers = $request->get_headers();
error_log( print_r( $_GET['themeName'], true ) );
error_log( print_r( $_GET['themeArgs'], true ) );
error_log( print_r( $_GET['file'], true ) );

This comment has been minimized.

@grappler

grappler Nov 1, 2017

Member

Can you remove the debug code?

This comment has been minimized.

@dingo-d

dingo-d Nov 1, 2017

Member

Will do.

@dingo-d

This comment has been minimized.

Member

dingo-d commented Nov 14, 2017

@grappler @ernilambar did you have the time to look at the PR? I would like to start with namespacing the plugin, and maybe adding some unit tests if necessary.

@grappler

This comment has been minimized.

Member

grappler commented Nov 15, 2017

@dingo-d Sorry for the delay. Thank you for working on this.

A couple a of comments and requests

  • Could we add the ajaxStopped text to the progress bar?
  • When choosing a new theme while a check is running hides the progress bar.
Stop check fixes
Add the message of the stopping in the progress bar, on theme switch the check will stop as well. Minor changes to the build process.
@dingo-d

This comment has been minimized.

Member

dingo-d commented Nov 15, 2017

@grappler Hi I made the changes you requested, should be better now. I made it so that when you switch the theme the checking will stop. Seems like ok behavior to me.

dingo-d added some commits Oct 24, 2017

Refactor js code, add linting, webpack and all that jazz
Need to change php files so that they work with the newly refactored js code.
JS and php fixes
Fixes that should accomodate the changes in the php code because of the changes in JS code. Minor build settings changes.
Changes to the code
Still need to fix quirks with files, add stop button (add listeners for start and stop for the ajax).
Fix the report add disable on button
Need to add the ajax stop functionality and make final refactor check

dingo-d added some commits Nov 15, 2017

Stop check fixes
Add the message of the stopping in the progress bar, on theme switch the check will stop as well. Minor changes to the build process.
Merge branch 'feature/rewrite-plugin-webpack' of github.com:dingo-d/t…
…heme-sniffer into feature/rewrite-plugin-webpack after rebasing the master branch for conflict fix
@dingo-d

This comment has been minimized.

Member

dingo-d commented Dec 28, 2017

@grappler I fixed all the collisions, I should I try to clean the commit history with interactive rebase?

@dingo-d dingo-d referenced this pull request Jan 24, 2018

Open

Sniff optimization #81

0 of 3 tasks complete

@dingo-d dingo-d self-assigned this Jan 24, 2018

@ernilambar

I am approving this PR. Lets not get stuck in this. If anything critical happens, we can always fix later.

@dingo-d dingo-d merged commit 522502d into WPTRT:master Jan 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dingo-d

This comment has been minimized.

Member

dingo-d commented Jan 26, 2018

Thanks! I'll get on with other work asap!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment