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

Improve api #5

Merged
merged 14 commits into from
May 7, 2017
Merged

Improve api #5

merged 14 commits into from
May 7, 2017

Conversation

turboMaCk
Copy link
Owner

@turboMaCk turboMaCk commented May 7, 2017

This is continuation of #4. Goals are:

  • refactor api to be cleaner and more conventional
  • refactor code to be cleaner and more conventional
  • ensure tests will fail when examples are incompatible
  • refactor examples
  • solve conditional subscriptions
  • update documentation
  • review once again

@turboMaCk
Copy link
Owner Author

@JonathanDDuncan you can follow this if you're interesting in further updates.

@turboMaCk
Copy link
Owner Author

Unfortunately tests should fail here due to multiple example incompatibility but they passes. Will be fixed.

Glue.subscriptions moves sub model
else
-- without this even clicks are bypassed
sub model
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok, this is really ugly with simple.

|> Glue.subscriptionsWhen .subModuleSubOn subModule
```
-}
subscriptionsWhen : (model -> Bool) -> Glue model subModel msg subMsg -> (model -> Sub msg) -> (model -> Sub msg)
Copy link
Owner Author

Choose a reason for hiding this comment

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

well not sure if this is good enough...

@turboMaCk turboMaCk mentioned this pull request May 7, 2017
5 tasks
@turboMaCk
Copy link
Owner Author

will continue by #6

@turboMaCk turboMaCk merged commit e884b5a into master May 7, 2017
@JonathanDDuncan
Copy link

@turboMaCk I decided to work on my own fork which has changed a lot from what I started with. It will give you an idea what I thought would be fun to write, which it has. Now I'm finished and may not touch it again for a while. But you are welcome to take a look and steal any ideas you feel like.
https://github.com/JonathanDDuncan/ElmMiniApps

@turboMaCk
Copy link
Owner Author

Pls don't use big letters in package name. elm-package has issues with that. Also be aware that some people might have issue with that OOP like api:

@turboMaCk
Copy link
Owner Author

Anyway I also see that you have it wrong. You're exposing module which isn't even there: https://github.com/JonathanDDuncan/ElmMiniApps/blob/master/elm-package.json#L10

@JonathanDDuncan
Copy link

@turboMaCk I decided last week to revisit my code and improve it a bit.
If you still feel that my API, is OOP like please let me know.

Basically now I am creating records that contains the references to the miniapps to be able to do everything and the code is just functions based on these. There are only two configurators: configure and configurebubbuling and both use the same API.

One thing that mine has different than Glue is that I can create a MiniAppsSimpleConfiguration of all the miniapps I want to expose with minimal changes to the main application itself and can add them anywhere to the view with MiniApps.viewsubapp miniapps miniapp model.

Instead of TEA I decided to go with miniapp.
https://groups.google.com/forum/#!searchin/elm-discuss/miniapp/elm-discuss/Lo6bG96zotI/uaLPmpSBDwAJ

Also I wanted to suggest that if the examples had something more substantial of a mini-app like a signin form instead of the counters it might not be as offensive to some of the early adopters of Elm.

Wish you the best

Jonathan

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

Successfully merging this pull request may close these issues.

None yet

2 participants