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 row condition for item detail #404

Merged
merged 2 commits into from Nov 9, 2016

Conversation

3 participants
@roman-vohnik
Copy link
Contributor

roman-vohnik commented Oct 31, 2016

Hi,

this PR allows to set callback as condition to render or not to render item detail for a specific row.

It is called directly on ItemDetail, are you ok with this aproach? Or should I rework it in a way like existing row conditions - new methods DataGrid::allowItemDetail(callable) and Row::hasItemDetail() ?

I like this way more, but I can be wrong.

@juniwalk

This comment has been minimized.

Copy link
Collaborator

juniwalk commented Nov 1, 2016

Looks good to me, please wait for @paveljanda to give you his opinion.

@paveljanda

This comment has been minimized.

Copy link
Member

paveljanda commented Nov 1, 2016

@roman-vohnik Could you please change the "render-condition" to for example "condition-callback"? Render condition is a bit misleading in my opinion. It does not render enything, it just enables/disables the item detail. Condition callback is also not as good as I want, but i have no idea how to name it better.

Anyone with better name?

@roman-vohnik

This comment has been minimized.

Copy link
Contributor Author

roman-vohnik commented Nov 1, 2016

"renderable_condition_callback" ?

@paveljanda

This comment has been minimized.

Copy link
Member

paveljanda commented Nov 1, 2016

@roman-vohnik render_condition_callback? :D

@paveljanda paveljanda merged commit e1ef021 into contributte:master Nov 9, 2016

2 checks passed

Scrutinizer 2 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@paveljanda

This comment has been minimized.

Copy link
Member

paveljanda commented Nov 9, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.