Skip to content

Conversation

@MichaelDeBoey
Copy link
Contributor

@MichaelDeBoey MichaelDeBoey commented Nov 22, 2019

Follow-up of #2768, #2670, #2671, #2328 & #3050

Things I did extra:

  • Change npmDependencyRemoved's signature to accept a string instead of { name: string }, because it only has 1 argument
  • Change externalResourceAdded's & externalResourceRemoved's signature to accept a string instead of { resource: string }, because it only has 1 argument
  • Move overmind subscriptions as close as possible to the components itself instead of passing it through
  • Type FontList's updateSearch function better
  • Make FontPicker, AddFont, ExternalFonts & ExternalResource a FunctionComponent
  • Just use a single file for ExternalFonts to not pollute the project files unnecessarily

@lbogdan
Copy link
Contributor

lbogdan commented Nov 22, 2019

Build for latest commit ac00327 is at https://pr3066.build.csb.dev/s/new.

Copy link
Contributor

@christianalfoni christianalfoni left a comment

Choose a reason for hiding this comment

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

Looks great! 😄 👍

Tested:

  • Add dependency
  • Remove dependency
  • Change dependency version
  • Add resource
  • Remove resource
  • Add font
  • Remove font

Tested that everything is actually added/removed

@christianalfoni christianalfoni merged commit 4e652e7 into codesandbox:master Nov 28, 2019
@MichaelDeBoey MichaelDeBoey deleted the overmind/Dependencies branch November 28, 2019 15:46
@MichaelDeBoey
Copy link
Contributor Author

Thanks! 🙂

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

Labels

🧠 Overmind Indicates that this is related to the app's State Management 🔨 Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants