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 functionality tr class in table #149

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

joserojasrodriguez
Copy link
Contributor

Add method for add custom class to tr on table.

@Gustavinho
Copy link
Owner

Gustavinho commented Feb 21, 2022

Hi @joserojasrodriguez thanks for raising this, I have some comments/questions

  • what would happen to all of our components? should we have to add the trClass() methods to all of them as you had to do to the mocked classes?
  • what do you need to achieve? publishing and overriding the component isn't working for you?

Thanks

@@ -51,7 +51,7 @@

<tbody>
@foreach ($items as $item)
<tr class="border-b border-gray-200 text-sm" wire:key="{{ $item->getKey() }}">
<tr class="border-b border-gray-200 text-sm {{$view->trClass($item)}}" wire:key="{{ $item->getKey() }}">
Copy link
Owner

Choose a reason for hiding this comment

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

With this approach, I think we are forcing all the components to have this method defined, in that case, there should be a validation to execute this only if it's defined, or maybe adding a default value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @Gustavinho this functionality is to be able to give a css class to a tr, for example

public funtion trClass($model)
{
   if($model->status == 'VA')
   {
     return 'bg-green';
    }
  
    return '';
}

Copy link
Contributor Author

@joserojasrodriguez joserojasrodriguez Feb 21, 2022

Choose a reason for hiding this comment

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

The tables are only in the table component so I think the other components would not be affected.

Although that method returns an empty string anyway

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks @joserojasrodriguez , I see what you're trying to do, it's cool, by "components" I meant "views" or Livewire components, I think if we don't define that trClass() method in our table view files, it will throw an error, even if we don't need to customize the tr class, this method will have to be there, I saw you added it in the pub file but what happens with the current projects and files already created?, we will have to add that method to all of our files, even returning an empty string, you had to do it with the mocked table views for testing, this is for retro compatibility with views already created, what I'm trying to say is that this method shouldn't be mandatory.

@@ -30,4 +30,14 @@ class DummyClass extends TableView
{
return [];
}

/**
Copy link
Owner

Choose a reason for hiding this comment

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

I think it would be better to remove this from the stub file and add it to the documentation

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.

2 participants