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

Try @wordpress/scripts #358

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

jffng
Copy link

@jffng jffng commented Aug 8, 2024

This PR migrates the build scripts to @wordpress/scripts to compile the theme as is. The idea is to simplify the build configuration, and provide an incremental change to allowing the blocks to be upgraded.

Ref: #285

Steps to test:

  1. cd private
  2. rm -rf node_modules
  3. yarn && yarn build
  4. Verify the theme scripts and assets load and work as expected

Considerations:

I think the main benefit to this PR is it cuts down on the projects development dependencies. But if you do go ahead with moving the blocks to a separate plugin in the near term, then maybe this PR is not as relevant or useful.

@@ -6,33 +6,35 @@ let subMenus = [];
// if menu has lost focus inadvertently, restore it
const setupFocusTrap = () => {
const lastMenuItem = pageHeader.querySelector('.mobile-menu > ul > li:last-of-type');
const menuItemClassList = `.${Array.from(lastMenuItem.classList).join('.')}`;
let previousFocus;
if ( lastMenuItem ) {
Copy link
Author

Choose a reason for hiding this comment

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

This is kind of an unrelated change, but I was getting a scripting error since this was undefined in my test setup.

@@ -114,7 +114,8 @@ class DisplayAuthor extends Component {

api
.getPostsFromAuthors(requestArguments, value)
.then((data = [], i, xhr) => { // eslint-disable-line
.then((data = [], i, xhr) => {
Copy link
Author

Choose a reason for hiding this comment

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

These changes were the result of running yarn lint --fix.

@jffng jffng force-pushed the try/wordpress-scripts branch from e8dff12 to 4e1d2d8 Compare August 9, 2024 17:08
@jffng jffng marked this pull request as ready for review August 9, 2024 21:18
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.

1 participant