Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant