Skip to content

Conversation

@mikeethedude
Copy link
Contributor

@mikeethedude mikeethedude commented Jun 17, 2024

Summary

This is where we are starting for 2.x Here are some of the thigs we need to accomplish in this branch.

  • Remove eslint
  • Update or remove chalk
  • Migrate or rebuild as a module type package
  • Update testing requirements
  • Improve Help Text
  • Init command should be interactive instead of failing with missing parameters.
  • Fix github actions checks to conform with new stuff

Documentation update (required)

If this pull request requires a change to Emulsify documentation, those changes, updates, and/or new information must accompany this pull request.

How to review this pull request

  • Pull this code
  • Take note of node version
  • npm run build
  • npm run test - All tests should pass
  • npm link
  • Go to a drupal instance and make sure the node version is the same
  • emulsify init Should take you through a wizard to install
  • go to the theme directory and emulsify system install emulsify-ui-kit
  • Should have UI Kit installed
  • Try and install a component emulsify component install text --force
  • Should install without error!

Closing issues

Closes #

@mikeethedude
Copy link
Contributor Author

Holy cow this was hard to get committed. I had to ignore these tests for now. We must clean these up before this is ready:
init.test.ts log.test.ts jsonSchemaToTs.test.ts getInitSuccessMessageForPlatform.test.ts getPlatformInfo.test.ts getAvailableStarters.test.ts

@mikeethedude mikeethedude marked this pull request as ready for review September 25, 2024 18:17
@joetower joetower self-requested a review September 27, 2024 13:16
Copy link

@joetower joetower left a comment

Choose a reason for hiding this comment

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

@mikeethedude Should I be able to run:
emulsify component install blockquote --force

And have it, or any other component (https://github.com/emulsify-ds/emulsify-ui-kit/tree/main/src/components) I list, install successfully?

I tried installing the blockquote and I get no feedback

Screenshot from 2024-09-27 09-25-52

@mikeethedude
Copy link
Contributor Author

@mikeethedude Should I be able to run: emulsify component install blockquote --force

And have it, or any other component (https://github.com/emulsify-ds/emulsify-ui-kit/tree/main/src/components) I list, install successfully?

I tried installing the blockquote and I get no feedback

Screenshot from 2024-09-27 09-25-52

@joetower Only if that is defined here: https://github.com/emulsify-ds/emulsify-ui-kit/blob/main/system.emulsify.json
@robherba We should probably change the output to something helpful if we can't find the definition for that component.

@joetower
Copy link

@mikeethedude Okay, thank you!

The thing is, in testing this, those three already exist, which, I suppose is why the --force is needed. So, to properly test I should check the contents of text before running emulsify component install text --force and then after?

@mikeethedude
Copy link
Contributor Author

Yeah you can try and remove that component or otherwise mess it up to see if that puts the right one in.

@joetower
Copy link

@mikeethedude Thank you! I removed text and tried to re-install and this is what the cli returned

Screenshot from 2024-09-27 12-40-38

Note: it did add the text component, but also returned an unexpected links mention

@joetower
Copy link

@mikeethedude But, if I remove it again and run it with --force it does what you'd expect.

Screenshot from 2024-09-27 12-42-24

@mikeethedude
Copy link
Contributor Author

I believe that lines up with the expectations though I think it should be okay with a dependency existing. Maybe we should have it ask if you want to replace the dependency when it already exists? The red text is also probably a little strong there.

This is good feedback, thank you!

@joetower joetower self-requested a review September 27, 2024 17:48
Copy link

@joetower joetower left a comment

Choose a reason for hiding this comment

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

@mikeethedude This tests as you've outlined. Super excited about this release and what will come next. 👍

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

Labels

None yet

Projects

No open projects
Status: In progress

Development

Successfully merging this pull request may close these issues.

4 participants