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

Add Button component #49 #110

Closed

Conversation

anilmaurya
Copy link
Contributor

No description provided.

@davidgoli
Copy link
Collaborator

2 things:

  1. Tests are failing
  2. Why do you want to add this functionality to the infinity-loader component? Its job is currently pretty well defined: scroll the loader into visibility, and a loadMore action is fired. Since you've created buttonMode to bypass this functionality, it seems that what you really want is a custom component. You can use the route loading magic of Ember Infinity without using the InfinityLoader component at all. It wouldn't be that hard to create actually:

template.hbs:

<ul class="test-list">
{{#each model as |item|}}
  <li>{{item.name}}</li>
{{/each}}
</ul>

{{load-more-button}}

load-more-button.js:

export default Ember.Component.extend({
  tagName: 'button'
  action: 'infinityLoad'
});

That should do it I think. Unless I'm missing something?

@anilmaurya
Copy link
Contributor Author

@davidgoli Thank you for taking a look.

Tests are failing for ember-1.10 and ember-beta, I will look the reason behind the failure.

Reason I added this functionality is because there is an open issue for adding button component, please check #49 and #46 .

@davidgoli
Copy link
Collaborator

I see, this is fine to add, though it seems a bit extraneous since it's so simple to roll your own. I'd still say it doesn't belong in the infinity-loader component IMO - since what it is is basically a completely different component that doesn't really share any functionality with infinity-loader. SRP, and all.

@kellyselden
Copy link
Collaborator

@davidgoli I think I would agree with you. This functionality seems better suited as a dependent add-on IMO, something like ember-infinity-button.

@hhff
Copy link
Collaborator

hhff commented Dec 7, 2015

I Agree - I think it's a different concern. I think it would be more important to add a note to the readme that explains how to use the infinityLoad action with a button rather than the infinity-loader

I think @davidgoli example would be great - but it would be nice to show a loading text on the button like we do with the infinity-loader. Devs will be able to go from there 👍

@anilmaurya
Copy link
Contributor Author

@hhff Should I send a pull request updating README(how to use ember-infinity with button) ?

you should close #49 as this is no longer required.

@hhff
Copy link
Collaborator

hhff commented Dec 8, 2015

@anilmaurya yes please! I updated #49 👍

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.

4 participants