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

Yield to inverse when tbody rows are empty #608

Merged
merged 2 commits into from
Dec 12, 2018

Conversation

ynotdraw
Copy link
Contributor

@ynotdraw ynotdraw commented Aug 8, 2018

One thing I noticed with this implementation is that since the yielded inverse content is rendered inside of the tbody, the content only spans the width of the first column by default. Any thoughts on the best way to solve this? Should it be wrapped in a row or something?

I'm not sure how you wanted to handle documentation either. I could take a stab, if you want to point me in the right area where you think it makes sense (maybe under the rows-and-trees section? Does it deserve it's own section?).

Looks like I may need to do something for Ember 1 as well (SUPPORTS_INVERSE_BLOCK).

@ynotdraw ynotdraw force-pushed the tbody-yield-to-inverse branch from a5c795c to a7bd2e4 Compare August 8, 2018 20:07
@chrismllr
Copy link

+1 here! this would be great, to expose a "No results" state.

@pzuraq
Copy link
Contributor

pzuraq commented Aug 24, 2018

Discussed this with @ynotdraw and I think it's a great feature, definitely want to get it merged!

It looks like there's a failure on the oldest Ember version we support. I can try to take a look, but I'm unsure when I'll have time. Once tests are passing there we can merge this.

@ynotdraw
Copy link
Contributor Author

I'm hoping to have some time this weekend to dig into the changes required to support the oldest Ember version with this PR 👍

@ynotdraw ynotdraw force-pushed the tbody-yield-to-inverse branch 2 times, most recently from 5a954f8 to 4d081d8 Compare November 9, 2018 19:33
@ynotdraw ynotdraw force-pushed the tbody-yield-to-inverse branch from 4d081d8 to 17b2f0a Compare December 3, 2018 15:25
@ynotdraw
Copy link
Contributor Author

ynotdraw commented Dec 3, 2018

@pzuraq howdy! I had some time to get back to these tests and support for Ember 1.x if you have some time to review!

Copy link
Contributor

@pzuraq pzuraq left a comment

Choose a reason for hiding this comment

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

Looks good to me, if @twokul is ok with this PR I think it’s ready to merge!

@twokul
Copy link
Contributor

twokul commented Dec 12, 2018

does it solve #631 or am I misreading the code?

@ynotdraw
Copy link
Contributor Author

ynotdraw commented Dec 12, 2018

@twokul oh nice - at the time that issue didn't exist, but yup, this PR will take care of allowing else on tbody! 🎉

@twokul twokul merged commit 13a8cad into Addepar:master Dec 12, 2018
@ynotdraw ynotdraw deleted the tbody-yield-to-inverse branch December 12, 2018 19:35
@qpowell
Copy link

qpowell commented Dec 12, 2018

Thanks @twokul <3

@jeannettetang
Copy link

<3

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

Successfully merging this pull request may close these issues.

6 participants